GH-41988: [Go] Add FormatRecoveredError to consistently handle recovery with wrapped errors #41989
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
jmacd
added a commit
to open-telemetry/otel-arrow
that referenced
this pull request
Jun 5, 2024
Part of #210. This updates memory-limit error handling because, prior to this fix, there were two fallbacks. (1) the use of os.Stderr to print the message that we had lost (2) a test for the expected number of records to match The first is now removed (i.e. no printing to os.Stderr). The second is now an internal error. The consumer is expected to test for the memory limit error explicitly (e.g., and handle it as ResourceExhausted). A new function is added to do this (NewLimitErrorFromError) that parses the message using a regexp. An upstream issue report and PR will be filed with Arrow-Go to overcome this in the future. See apache/arrow#41989.
zeroshade
approved these changes
Jun 7, 2024
github-actions
bot
added
awaiting merge
|
It looks like you need to re-run |
|
@zeroshade Thank you! |
|
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 4df00fa. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them. |
jmacd
added a commit
to jmacd/otel-arrow
that referenced
this pull request
Jul 16, 2024
Remove a pre-v17 hack that was fixed by apache/arrow#41989.
jmacd
added a commit
to open-telemetry/otel-arrow
that referenced
this pull request
Jul 16, 2024
Remove a pre-v17 hack that was fixed by apache/arrow#41989. See open-telemetry/opentelemetry-collector-contrib#34114.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rationale for this change
Part of #36186 (Fixes it, IMO)
Fixes #41988.
What changes are included in this PR?
A new internal
FormatRecoveredError()function adds consistency to errors generated from panic recovery.Are these changes tested?
Yes.
Are there any user-facing changes?
Users will be able to unwrap errors thrown by allocators, for example.