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

sqlccl: log scatter errors in RESTORE #15047

Merged
merged 1 commit into from
Apr 18, 2017
Merged

Conversation

danhhz
Copy link
Contributor

@danhhz danhhz commented Apr 18, 2017

This was a useful debugging aid when trying to figure out why scatter
didn't seem to be working.

This was a useful debugging aid when trying to figure out why scatter
didn't seem to be working.
@danhhz danhhz requested a review from dt April 18, 2017 17:58
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@dt
Copy link
Member

dt commented Apr 18, 2017

:lgtm:

Though (unrelated to this change) as with any logged-then-ignored error, I worry about it regressing unnoticed -- I wonder if could e.g. hook log.Warning in tests to t.Fatal or something.


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved, some commit checks pending.


Comments from Reviewable

@danhhz
Copy link
Contributor Author

danhhz commented Apr 18, 2017

failing the test when this happened would make it flaky. I made an issue to fix the best-effort nature of scatter, which means this could be made a failing error: #15049


Review status: 0 of 1 files reviewed at latest revision, all discussions resolved, some commit checks pending.


Comments from Reviewable

@danhhz danhhz merged commit bcfbbd8 into cockroachdb:master Apr 18, 2017
@danhhz danhhz deleted the scatterlog branch April 18, 2017 18:14
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