Thanks, Tobias, for digging into this and putting together the PR. Thanks, Robert, for clarifying the intended behavior.
Making the retry settings configurable makes sense to me. We should also make sure the documented MongoDB timeouts and retry budget work together, so a single stalled attempt doesn't consume the entire budget. Yufei On Tue, Oct 13, 2026 at 5:57 AM Tobias Riemenschneider <[email protected]> wrote: > > Hi, > > I have filed the issue https://github.com/apache/polaris/issues/5777 and > prepared the according PR https://github.com/apache/polaris/pull/5779. > > Regards, > Toby > > Am Di., 13. Okt. 2026 um 13:29 Uhr schrieb Robert Stupp <[email protected]>: > > > Hi, > > > > Tobias' observation is correct. > > RetryConfig was always meant to be configurable, it already has the > > necessary annotations, but it effectively is not, as > > PersistenceParams.retryConfig() is a Java default function, which SmallRye > > config "ignores." > > > > I wouldn't mind seeing the PersistenceParams interface changed (aka > > retryConfig() not being a default function, while > > BuildablePersistenceParams.retryConfig() can stay a default function). > > I'm happy to review a change for that. > > > > Robert > > > > > > On Tue, Oct 13, 2026 at 9:28 AM Jean-Baptiste Onofré <[email protected]> > > wrote: > > > > > Hi Tobias, > > > > > > Great analysis and thanks for the reproducer in TestRetryLoopImpl. > > > > > > It seems you found some kind of inconsistency :) > > > > > > I don't see an intentional reason for the NoSQL retry config to be fixed > > > (I'm sure Robert has some insights about that). JDBC already exposes > > > max-retries, max-duration and initial-delay, so exposing the same in > > NoSQL > > > would be consistent. I think a PR would make sense for that. > > > > > > Having 30s for read-timeout and server-selection-timeout with 15s retry > > > budget means we can have a stalled attempts without retry (as you > > > described). I would treat that as a doc "issue" :) Once the budget is > > > configurable, we can align the defaults and document. > > > > > > I'm not sure we should change the per-attempt deadline: it basically > > > changes the retry logic, it's not just a configuration update. I believe > > > the Mongo driver already has its own timeouts (bound to an attempt). I > > > would suggest to keep it out of the config PR and discuss it > > > separately. @Robert, as you know better than me the NoSQL retry design, > > > could you weigh in on whether the total-budget-only model was > > intentional? > > > > > > Anyway, I'm happy your clients retry successfully and nothing is lost :) > > > > > > Regards > > > JB > > > > > > On Mon, Oct 12, 2026 at 1:01 PM Tobias Riemenschneider < > > > [email protected]> wrote: > > > > > > > Hi all, > > > > > > > > We run Polaris 1.8.0 with the NoSQL (MongoDB) persistence and saw the > > > > pattern below on 1.7.0; the relevant code is unchanged in 1.8.0 and > > main. > > > > I'd like to check it against the intended design before filing an > > issue. > > > > > > > > *What we see:* when MongoDB stalls for about 15 s (for example > > w:majority > > > > writes behind a lagging secondary), table commits fail with > > > > > > > > RetryTimeoutException: Retry timeout after PT15.002S, 0 retries > > > > > > > > and clients get a 500. The commits are retried by our client and no > > data > > > is > > > > lost, so this is low impact for us. > > > > > > > > *Why, as far as I can tell:* RetryLoopImpl checks the total budget ( > > > > RetryConfig.timeout, default PT15S) only after an attempt has ended, > > and > > > > nothing bounds a single attempt. A stalled first attempt therefore > > leaves > > > > no budget for a second one. I reproduced this with a unit test in > > > > TestRetryLoopImpl on main (simulated clock, default config). The > > > relational > > > > JDBC withRetries has the same single-attempt behaviour, so I assume > > that > > > > part is by design. I checked this comparison and the config reference > > > > against both main and the 1.8.0 tag. > > > > > > > > *Two things look inconsistent to me:* > > > > > > > > 1. The NoSQL retry config cannot be changed. I found no > > > > polaris.persistence.retry* property in the config reference, and > > > > PersistenceParams.retryConfig() returns the constant default, while > > > the > > > > JDBC persistence exposes max-retries, max-duration-in-ms and > > > > initial-delay-in-ms. > > > > 2. The helm docs recommend quarkus.mongodb.read-timeout: 30S and > > > > server-selection-timeout: > > > > 30S. Both are longer than the 15 s budget, so with the documented > > > > settings a stalled attempt can only end after the budget is gone. > > > > > > > > > > > > *Questions:* > > > > > > > > - Is the retry config intentionally not configurable? > > > > - Is the 30 s driver timeout recommendation meant to go together > > with > > > > the 15 s budget, or should one of them change? > > > > - Would a per-attempt deadline (a fraction of the total budget) be > > > > welcome, or is the total-budget-only model intended? > > > > > > > > > > > > If exposing the retry config as properties (like JDBC) is acceptable, > > I'm > > > > happy to send a PR. > > > > > > > > I used AI assistance to analyse the code and draft this message; I have > > > > reviewed it. > > > > > > > > Best regards, > > > > > > > > *Tobias Riemenschneider* > > > > > > > > Sr. Lead Software Engineer > > > > > > > > [email protected] > > > > Cumulocity GmbH, Software Center 5b, 35037 Marburg, Germany > > > > < > > > > > https://www.google.com/maps/search/Software+Center+5b,+35037+Marburg,+Germany?entry=gmail&source=g > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > > > > > > > Website <https://www.cumulocity.com> | Linkedin > > > > <https://www.linkedin.com/company/cumulocity/> > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Website <https://www.cumulocity.com> | Linkedin > <https://www.linkedin.com/company/cumulocity/> > > >
