Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storage: fix RocksDB error and go os error discrepancy #26259

Merged
merged 1 commit into from
May 31, 2018

Conversation

windchan7
Copy link
Contributor

Because error returned from RocksDB is different from go's os error, we need to
bridge the gap by examining error message.

Fixes #26121
Release note: None

Because error returned from RocksDB is different from go's os error, we need to
bridge the gap by examining error message.

Release note: None
@windchan7 windchan7 requested review from tbg and a team May 30, 2018 21:28
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg
Copy link
Member

tbg commented May 31, 2018

I'll merge this so that the test flakes go away.

Please add a unit test in a new PR that makes an env, and exercises these two errors by reading nonexistent files.

Please also visit https://crdb.io/dashboard/?filter=l%3AA-core+c%3Atest-failure&group=repo+assignee&sort=-updated&display=category+severity+updated and close the other issues assigned to you from this bug as well with a reference to this PR. Thanks!

bors r+

craig bot pushed a commit that referenced this pull request May 31, 2018
26259: storage: fix RocksDB error and go os error discrepancy r=tschottdorf a=windchan7

Because error returned from RocksDB is different from go's os error, we need to
bridge the gap by examining error message.

Fixes #26121 
Release note: None

Co-authored-by: Victor Chen <[email protected]>
@craig
Copy link
Contributor

craig bot commented May 31, 2018

Build succeeded

@craig craig bot merged commit f07c7f0 into cockroachdb:master May 31, 2018
This was referenced May 31, 2018
@windchan7
Copy link
Contributor Author

Flaky test issues are closed. Will work on the unit test. Thanks, Tobi.

windchan7 added a commit to windchan7/cockroach that referenced this pull request May 31, 2018
Add a unit test to test `OpenFile`, `ReadFile`, and `DeleteFile` in RocksDB
return `os.ErrNotExist` error when the file or directory is not found.

Related to: cockroachdb#26259.
Release note: None
windchan7 added a commit to windchan7/cockroach that referenced this pull request May 31, 2018
Add a unit test to test `OpenFile`, `ReadFile`, and `DeleteFile` in RocksDB
return `os.ErrNotExist` error when the file or directory is not found.

Related to: cockroachdb#26259.
Release note: None
craig bot pushed a commit that referenced this pull request May 31, 2018
26289: storage: add a unit test to test RocksDB's error message is expected r=windchan7 a=windchan7

Add a unit test to test `OpenFile`, `ReadFile`, and `DeleteFile` in RocksDB
return `os.ErrNotExist` error when the file or directory is not found.

Related to: #26259.
Release note: None

Co-authored-by: Victor Chen <[email protected]>
craig bot pushed a commit that referenced this pull request May 31, 2018
26239: CONTRIBUTING.md: avoid recommendation to use 'go get' r=couchand a=benesch

Since #25325 landed, 'go get' no longer works. Adjust our contributing
guidelines appropriately.

Release note: None

/cc @rendaw (thanks for the report!)

26290: sql, opt: Add pg error codes, tests for error messages r=rytaft a=rytaft

This commit ensures that all errors in the optbuilder have a pg
errorcode. In order to keep the optimizer in sync with the heuristic
planner, this commit also updates some of the heuristic planner error
messages to match the Postgres error messages and include pgcodes.

Other errors in the optbuilder which should never be thrown (e.g., in
the default case of a switch statement that covers all cases) have been
converted into panics.

Release note (sql change): Fixed some error messages to more closely
match the Postgres error messages, including the corresponding Postgres
error codes.

26296: importccl: re-enable TestImportCSVStmt r=mjibson a=mjibson

The underlying bug appears to have been fixed in #26259.

Release note: None

26298: Makefile: fix check-libroach with linux release toolchain r=mberhault a=benesch

The Linux release toolchain uses an ancient version of glibc that
requires linking against -lrt. Teach CMake to link test binaries
accordingly on Linux.

The Linux release toolchain also uses a very recent version of libstdc++
that must be statically linked. Plumb the link flags down into CMake via
the LDFLAGS variable. (Previously they were only passed down to links
invoked by Go via the -extldflags flag in the LINKFLAGS variable.)

Release note: None

Co-authored-by: Nikhil Benesch <[email protected]>
Co-authored-by: Rebecca Taft <[email protected]>
Co-authored-by: Matt Jibson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants