apache / accumulo

Apache Accumulo
https://accumulo.apache.org
Apache License 2.0
1.07k stars 445 forks source link

Improve tablet server batch reader iterator time tracking code #4927

Open DomGarguilo opened 1 month ago

DomGarguilo commented 1 month ago

This pull request introduces the CountDownTimer class to replace the existing Timer class for better handling of timeout and retry logic in the TabletServerBatchReaderIterator class. The most significant changes include the integration of CountDownTimer in various methods and the addition of new methods to the CountDownTimer class to support the new functionality.

Integration of CountDownTimer:

Enhancements to CountDownTimer class:

dlmarion commented 1 month ago

I think I missed the initial inclusion of the CountDownTimer class. I went looking to see if an implementation already existed that we could just use and not have to maintain ourselves. I didn't find one, but I found three implementations of a StopWatch. One StopWatch is in our codebase and it appears unused and a candidate for removal, Commons-Lang has one, and Guava does too. A timer is just checking to see if some amount of time has passed. I'm wondering if we should leverage an existing StopWatch implementation inside of the CountDownTimer to reduce the amount of code that we have to maintain and test.

ctubbsii commented 1 month ago

I think it's worth looking into one of the existing libraries to use. But, we'd need to be careful it's fast and efficient, and not something that is going to be slow by creating unnecessary Duration objects to do simple arithmetic operations on nanoTime.

DomGarguilo commented 1 month ago

Yea I think its worth looking into as well. Maybe we can open a new ticket to remove the StopWatch class thats in our codebase and look into the existing timing utils.