Hi Robert, > Isn't it okay to yield a HTTP/409 in this particular case (namespace properties) and let the user figure out the right values?
Unfortunately, I do not think it would be ok. AFAIK, current Iceberg java code interprets 409 in this case as "namespace already exists" [1]. I'm personally ok without retries on Namespace update collisions, however, I'd prefer to respond with 429 (Too Many Requests) in that case (as I mentioned earlier). [1] https://github.com/apache/iceberg/blob/b1c8bc589e0caae3d0a1649e354c44d0fb23759c/core/src/main/java/org/apache/iceberg/rest/ErrorHandlers.java#L184-L185 Cheers, Dmitri. On Thu, Jul 31, 2025 at 11:03 AM Robert Stupp <sn...@snazy.de> wrote: > Having retries would be great. > > However, for namespace property updates, the situation is undefined, > because there are no "prerequisites" that have to be satisfied. > Say, you have two concurrent requests: > - One requests sets a=b > - Another request removes a > - Yet another request sets a=c > Technically it's perfectly valid to eventually have a=c, a=b or no a > in the namespace properties. > > I think concurrent property updates on namespaces are rather a rare > situation - but difficult to implement right. Is there a "right way", > considering there is no defined behavior? > Isn't it okay to yield a HTTP/409 in this particular case (namespace > properties) and let the user figure out the right values? > > Another aspect is how retries are implemented and configured, what are > reasonable default retry config options (number of retries, retry > interval, overall timeout, jitter, fair/unfair, etc). Spoiler: there's > already an implementation [1] heavily inspired by Nessie. > I'm all in to have a well defined and working retry mechanism for all > applicable requests, but it should be one mechanism. There is already > one specialized retry mechanism in > o.a.p.service.catalog.iceberg.IcebergCatalog, and a different > specialized one in > o.a.o.persistence.relational.jdbc.DatasourceOperations. > > API-level operations (think: something like a table update) "drive" > the total request timeout and all individual dependent operations > should ideally respect that. That's rather a mid/long term vision > though. > > [1] > https://github.com/snazy/polaris/tree/persistence-nosql/persistence/nosql/persistence/impl/src/main/java/org/apache/polaris/persistence/nosql/impl/commits/retry > + the parent package. > > On Tue, Jul 22, 2025 at 7:44 PM Dmitri Bourlatchkov <di...@apache.org> > wrote: > > > > Heads up: PR [1989] is proceeding without retries and with 409 response > codes. > > > > Please review if you have an opinion. > > > > [1989] https://github.com/apache/polaris/pull/1989 > > > > Thanks, > > Dmitri. > > > > On 2025/07/16 15:15:19 "Rizzo Cascio, Fabio" wrote: > > > Hi all, > > > > > > > > > > > > While working on Issue #1390< > https://github.com/apache/polaris/issues/1390>, what initially seemed > like a quick fix has turned into a more complex problem. Following > Dimas-b's suggestion, I'm opening this up for a wider discussion. > > > > > > You can see the comments on my PR #1989< > https://github.com/apache/polaris/pull/1989>. > > > > > > I have a couple of questions: > > > > > > 1. If multiple property updates are coming in concurrently and they > don't update the same key, should we merge the properties (entity coming in > the request and the entity refreshed) and make the update? > > > 2. If multiple updates are coming in concurrently and they are > trying to update the same property key value, should we throw an exception? > What error code should we return? > > > > > > I look forward to your thoughts. > > > > > > > > > > > > Thanks, > > > > > > > > > > > > Fabio > > > > > > This message is confidential and subject to terms at: > https://www.jpmorgan.com/emaildisclaimer including on confidential, > privileged or legal entity information, malicious content and monitoring of > electronic messages. If you are not the intended recipient, please delete > this message and notify the sender immediately. Any unauthorized use is > strictly prohibited. > > > >
