Skip to content

Use Duration for Fate.shutdown and EventQueue.poll - #6642

Open
DomGarguilo wants to merge 1 commit into
apache:mainfrom
DomGarguilo:accumulo-4361-duration-core
Open

DomGarguilo wants to merge 1 commit into
apache:mainfrom
DomGarguilo:accumulo-4361-duration-core

Conversation

@DomGarguilo

Copy link
Copy Markdown
Member

Closes #4361

Changes

  • Fate.shutdown(long, TimeUnit) → Fate.shutdown(Duration). The shutdown wait chain (waitForAllFateExecShutdown, waitForDeadResCleanerShutdown, FateExecutor.waitForShutdown) now passes a single CountDownTimer instead of (start, timeout, unit).
  • EventQueue.poll(long, TimeUnit) → EventQueue.poll(Duration). Callers use a static final constant.
  • CountDownTimer.startNew(Duration) now caps very large durations (e.g. Duration.ofMillis(Long.MAX_VALUE)) at Long.MAX_VALUE nanos instead of throwing ArithmeticException. This matches the long, TimeUnit overload and Timer.hasElapsed(Duration), which already do this through Timer.toNanos. That helper is now package-private and reused here. Documented in the javadoc and covered by a new test.

Reasoning

  • Fate shutdown: Runs once per shutdown, so allocation doesn't matter. The deadline is now one object, and the loops read !countdown.isExpired() instead of repeating System.nanoTime() - start < unit.toNanos(timeout). A raw long start also no longer has to be a nanoTime value by convention.
  • EventQueue.poll: Called about every 100ms, but callers pass a static final constant, so there are no new allocations. Gains a stronger type and a named constant.
  • CountDownTimer capping: Makes the Duration and long, TimeUnit overloads behave the same, so callers that use Long.MAX_VALUE ms to mean "no timeout" (e.g. TabletServerBatchReaderIterator) can switch to Duration without gaining a crash path.

Co-Authored-By: Claude Opus 5.5

@DomGarguilo DomGarguilo added this to the 4.0.0 milestone Nov 9, 2026
@DomGarguilo DomGarguilo self-assigned this Nov 9, 2026
@DomGarguilo

Copy link
Copy Markdown
Member Author

This is the last batch and will close #4361 once its is merged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate replacing long+TimeUnit with Duration in more places

1 participant