Core: Use Shared HttpClientContext to Persist "was-retried" Attribute
Summary
Fixes an issue where retry flag was-retried was not accessible after request execution due to context isolation.
Previously, requests were executed with a BasicHttpContext instance, which caused Apache HttpClient to create internal child contexts. As a result, any attributes (such as was-retried) set during the retry process were not visible to the caller after the response.
This change ensures that a shared HttpClientContext is used when executing requests, allowing retry-related attribute to persist and be accessible for error handling and observability.
Prior work: #12818
Changes
Use HttpClientContext.create() instead of BasicHttpContext.
Testing
Adding a unit test that simulates a self-conflict table corruption scenario.
@XJDKC We may be going a different route with this, there were some offline discussions and we are considering just disabling all retries to just avoid this issue entirely and just throw CSU for all 5XX errors
@XJDKC We may be going a different route with this, there were some offline discussions and we are considering just disabling all retries to just avoid this issue entirely and just throw CSU for all 5XX errors
Got it, so we're planning to retry only when the error code is 429?
Just realized that this PR is to fix the HttpContext creation, so we still need to check in this fix but it's not a required fix for the protential table corruption.
cc: @RussellSpitzer
given that the was-retried flag is being removed in https://github.com/apache/iceberg/pull/13352 is this PR still needed?
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the [email protected] list. Thank you for your contributions.