IndexingMerger: lessen work and retry by default #3732
Conversation
Catching an exception, the IndexingMerger attempts to detect if it should lessen work and retry. Currently it is an include list of known "retyable" errors. If the error is not detected correctly, or not predicted, it will not retry. Being a background process, however, it is better and safer to retry by default. Resolves FoundationDB#3731
| if (ex == null) { | ||
| return false; | ||
| } | ||
| final Set<Integer> lessenWorkCodes = new HashSet<>(Arrays.asList( |
There was a problem hiding this comment.
This set can be declared as a static instance - no need to instantiate every time
There was a problem hiding this comment.
Just wonder - wouldn't the JVM do the right thing here? It seems like an obvious optimization and I know that other languages are performing this optimization (and much more).
...r-core/src/main/java/com/apple/foundationdb/record/provider/foundationdb/IndexingMerger.java
Outdated
Show resolved
Hide resolved
| // Expecting AsyncToSyncTimeoutException or an instance of TimeoutException. However, cannot | ||
| // refer to AsyncToSyncTimeoutException without creating a lucene dependency | ||
| // TODO: remove this kludge | ||
| if (e.getClass().getCanonicalName().contains("Timeout") ) { |
There was a problem hiding this comment.
From the PR description:
Being a background process, however, it is better and safer to retry by default.
Why not just have this return false if there is any exception, except InterruptedException.
Here you abort for any exception that doesn't have Timeout in its name, or an FDBException as its cause.
Either way, it seems like having a test with a mock IndexMaintainer that has various failure scenarios would be valuable.
3 participants
Catching an exception, the IndexingMerger attempts to detect if it should lessen work and retry. Currently it is an include list of known "retyable" errors. If the error is not detected correctly, or not predicted, it will not retry.
Being a background process, however, it is better and safer to retry by default.
Resolves #3731