iceberg icon indicating copy to clipboard operation
iceberg copied to clipboard

REST: Revert #12818 and additionally stop retrying on 502/504

Open singhpk234 opened this issue 6 months ago • 0 comments

About the change

presently, we retry on 502 / 504 as well here, we have a spec definition stating here that when these status are thrown the commit status can be unknown, so says the RFC as something went wrong in the middle of processing the request. So we retry with 502 and 504 we can conflict with ourselves, as we don't know when we receive the status of 409 is it because we retried on 502 and then got 409 or something else happened, so it's better to throw the commit state unknown if we know the commit has been retried.

The Iceberg users who complained this was with 504 : slack thread - https://apache-iceberg.slack.com/archives/C025PH0G1D4/p1747992294134219

We have similar handling for glue as we did here https://github.com/apache/iceberg/pull/7198 as there are internal http retries over the glue SDK.

So I Added a PR considering above https://github.com/apache/iceberg/pull/12818

But based on offline discussions with more folks, according to the spec its best to not retry on 502 / 504 as it clearly calls out that the status is unknown.

The change attempts to :

  • revert https://github.com/apache/iceberg/pull/12818 [revert commit a5a58f0a14effa339caa66cdd17ef2dc2e34b7e8]
  • removes 502 and 504 from the retries.

Testing

Adding unit tests

cc @amogh-jahagirdar @rdblue @RussellSpitzer

singhpk234 avatar Jun 19 '25 00:06 singhpk234