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

cli: return server's errors in nodelocal upload #46311

Merged
merged 1 commit into from
Mar 19, 2020

Conversation

dt
Copy link
Member

@dt dt commented Mar 19, 2020

Previously the cli code for 'nodelocal upload' was using the retrying helper from cockroach-go
to run its COPY-based put of the file. Somehow this meant that the error the server was sending
back to the client was getting lost and instead the 'current txn is aborted' error was all that
was printed to the user. While I am still unclear on exactly where the original error went, it
seems that by using the go db/sql interface directly we see the expected errors, and as this
code is not running traditional OLTP/transactional SQL, it likely can skip the retry loop.

Fixes #46027.

Release justification: bug fix.

Release note (cli change): ensure the correct error messages are shown to the user when using 'nodelocal upload'.

@dt dt requested a review from knz March 19, 2020 13:56
@dt dt requested a review from a team as a code owner March 19, 2020 13:56
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea.

Previously the cli code for 'nodelocal upload'  was using the retrying helper from cockroach-go
to run its COPY-based put of the file. Somehow this meant that the error the server was sending
back to the client was getting lost and instead the 'current txn is aborted' error was all that
was printed to the user. While I am still unclear on exactly where the original error went, it
seems that by using the go db/sql interface directly we see the expected errors, and as this
code is not running traditional OLTP/transactional SQL, it likely can skip the retry loop.

Release justification: bug fix.

Release note (cli change): ensure the correct error messages are shown to the user when using 'nodelocal upload'.
@dt
Copy link
Member Author

dt commented Mar 19, 2020

bors r+

@craig
Copy link
Contributor

craig bot commented Mar 19, 2020

Build succeeded

@craig craig bot merged commit 6e991cb into cockroachdb:master Mar 19, 2020
@dt dt deleted the upload-err branch March 31, 2020 14:13
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.

cli (,sql?): nodelocal upload does not print actual error messages
3 participants