somandal commented on PR #15618: URL: https://github.com/apache/pinot/pull/15618#issuecomment-2852295027
> > Before this PR, if we set externalViewStabilizationTimeoutInMs=10000ms, regular run will fail after 10000ms wait, while bestEfforts continue the next step after 10000ms. > > In this PR, regular run will succeed after, for example, 150000ms (extended 14 times), while bestEfforts continue the next step after 10000ms. > > All of this is because current PR will not give any timeout extension to bestEfforts=true. What I suggested is to allow bestEfforts to have this dynamic timeout too. > > Conclusion from offline discussion: In the sense that `externalViewStabilizationTimeoutInMs` is likely to remain the same as before (not expect to tune this number), not giving timeout extension to `bestEfforts=true` is preferred so as to lower the impact to the current uses of bestEfforts, as they would just behave as usual. Discussed this more offline with @Jackie-Jiang and @klsince and we have consensus that we can change the behavior for bestEfforts=true to also extend the timeout as long as progress is made. If we don't make progress, bestEfforts=true should continue onto the next step rather than failing. Let's incorporate this behavior into this PR itself @J-HowHuang cc @yashmayya for FYI -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
