ddanielr commented on PR #3231: URL: https://github.com/apache/accumulo/pull/3231#issuecomment-1461472973
> Overall this looks okay, but removing the retry in favor of a wait loop looses some functionality - mainly that there was a small pause at start. It also seems cleaner because the wait times do not need to be calculated. Also, with the retry, there would be a most 3 log messages generated for the wait. > > Why did you choose not to use retry? I attempted using a Retry with an earlier version of this fix. However, while wait times do not need to be calculated with a Retry, the amount of retries in the given time duration window still need to be calculated. Otherwise it never completes and just keeps retrying. In my first iteration, the retry never matched up to my time value as the increment value would always cause the retry to overshoot the defined property value. https://github.com/ddanielr/accumulo/pull/5/commits/f93c327073d660039716e21330deb98986b19494#diff-56945d7261689b2323a668699ab5865a1e48ec8424b0d612091d29ae0f2fc67cR1510 Because of the complications with the Retry object, and the fact that`blockForTservers()` is not attempting an operation as it's just blocking, I chose to simplify it for a wait loop. I can add in a small wait at the start if you'd prefer, or I'm happy to dig further into the Retry object with someone and see if I'm just completely missing something. I'm not super concerned about the logs being spammed as this is a block on the main thread so nothing else should really be showing up in the logs until this method has completed. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
