iceberg icon indicating copy to clipboard operation
iceberg copied to clipboard

Core: Use Shared HttpClientContext to Persist "was-retried" Attribute

Open XJDKC opened this issue 7 months ago • 3 comments

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 avatar Jun 18 '25 02:06 XJDKC

@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

RussellSpitzer avatar Jun 18 '25 21:06 RussellSpitzer

@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?

XJDKC avatar Jun 18 '25 21:06 XJDKC

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

XJDKC avatar Jun 18 '25 23:06 XJDKC

given that the was-retried flag is being removed in https://github.com/apache/iceberg/pull/13352 is this PR still needed?

nastra avatar Jun 24 '25 14:06 nastra

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.

github-actions[bot] avatar Jul 25 '25 00:07 github-actions[bot]