Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

Commit

Permalink
Remove unused sleepForBackoff() (#3432)
Browse files Browse the repository at this point in the history
**Goals (and why)**:
- Fix #3402 (by removing the confusing method)

**Implementation Description (bullets)**:
- Remove the sleepForBackoff() method from `AbstractTransactionManager`, and calls to it when retrying

**Testing (What was existing testing like?  What have you done to improve it?)**:
- There wasn't a test that we backoff (but this is expected, since no implementations exist in the Atlas codebase, nor in internal repositories, looking at a code-search)

**Concerns (what feedback would you like?)**:
- Did I miss any places where this is actually used? I did a find-usages + a search for `sleepForBackoff()` on internal github.
- Should we support some kind of backoff?

**Where should we start reviewing?**: `AbstractTransactionManager.java`

**Priority (whenever / two weeks / yesterday)**: Whenever. This is more a cleanliness/hygiene thing.
  • Loading branch information
jeremyk-91 authored and tboam committed Aug 2, 2018
1 parent fd7f35b commit 17a282e
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ public <T, C extends PreCommitCondition, E extends Exception> T runTaskWithCondi
log.warn("[{}] RuntimeException while processing transaction.", SafeArg.of("runId", runId), e);
throw e;
}
sleepForBackoff(failureCount);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ public abstract class AbstractTransactionManager implements TransactionManager {
this.timestampValidationReadCache = timestampCache;
}

protected static void sleepForBackoff(@SuppressWarnings("unused") int numTimesFailed) {
// no-op
}

protected boolean shouldStopRetrying(@SuppressWarnings("unused") int numTimesFailed) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ private static HeldLocksToken acquireLock(LockService lockService, LockRequest l
log.warn("Failing after {} tries", failureCount, ex);
throw ex;
}
AbstractTransactionManager.sleepForBackoff(failureCount);
} else {
return response;
}
Expand Down
6 changes: 6 additions & 0 deletions docs/source/release_notes/release-notes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ develop
We now perform the operations correctly only considering the value (or absence of value) in the relevant cell.
(`Pull Request <https://github.com/palantir/atlasdb/pull/3388>`__)

* - |improved| |devbreak|
- We have removed the ``sleepForBackoff(int)`` method from ``AbstractTransactionManager`` as there were no known users and its presence led to user confusion.
AtlasDB does not actually backoff between attempts of running a user's transaction task.
If your service overrides this method, please contact the AtlasDB team.
(`Pull Request <https://github.com/palantir/atlasdb/pull/3432>`__)

* - |improved|
- Sequential sweep now sleeps longer between iterations if there was nothing to sweep.
Previously we would sleep for 2 minutes between runs, but it is unlikely that anything has changed dramatically in 2 minutes so we sleep for longer to prevent scanning the sweep priority table too often. Going forward the most likely explanation for there being nothing to sweep is that we have switched to targeted sweep.
Expand Down

0 comments on commit 17a282e

Please sign in to comment.