Online Indexer: stop retrying an unresolvable partly-built mismatch - #4454
Online Indexer: stop retrying an unresolvable partly-built mismatch#4454jjezra wants to merge 4 commits into
Conversation
Each policy adjustment in the CONTINUE path is now attempted at most once, and the mismatch reported to the caller is the first one - naming the requested indexing method rather than the internal fallback.
| fallbackToRecordsScan = true; | ||
| return indexingLauncher(indexingFunc, attemptCount); | ||
| } | ||
| if (method == IndexBuildProto.IndexBuildIndexingStamp.Method.BY_INDEX && |
There was a problem hiding this comment.
Why isn't the whole method like this. i.e Shouldn't we always just create a new policy based on the previous method and carry on.
| // to a single target, but not to a subset. | ||
| fallbackToRecordsScan = true; | ||
| return indexingLauncher(indexingFunc, attemptCount); | ||
| // Here: match the policy to the previous run. Every adjustment is attempted once |
There was a problem hiding this comment.
Why would an adjustment happen more than once in general? i.e if we get to this point because the previous method was by_index and we're doing multi_target, shouldn't it switch to by_index. At which point it wouldn't go back to multi_target, right?
The only reason I can see this happening more than once is if two indexers are running simultaneously and both are set to REBUILD with different methods, and thus keep stepping over each other, but that should be prevented by the heartbeats
There was a problem hiding this comment.
Looking at this a bit more... setIndexingTypeOrThrow checks shouldAllowTypeConversionContinue and if it is not allowed to takeover, it throws PartlyBuiltException. So that handles the case where we use our policy, and this is intended to handle the case where our fallback policy is the same?
| return indexingLauncher(indexingFunc, attemptCount); | ||
| } | ||
| if (method == IndexBuildProto.IndexBuildIndexingStamp.Method.BY_INDEX && | ||
| !isPolicySourceIndexOf(conflictingIndexingTypeStamp)) { |
There was a problem hiding this comment.
Ig this is true, doesn't that mean we're rebuilding by the same method, and thus we should not be catching IndexingBase.PartlyBuiltException ?
| } | ||
|
|
||
| private boolean isPolicySourceIndexOf(IndexBuildProto.IndexBuildIndexingStamp conflictingIndexingTypeStamp) { | ||
| // true if the policy already points at the conflicting stamp's source index, hence retrying by this source |
There was a problem hiding this comment.
Do you have coverage of trying to build with a different source index?
|
|
||
| @ParameterizedTest | ||
| @BooleanSource | ||
| void testMultiTargetPartlyBuiltContinueByIndex(boolean allowTakeover) { |
There was a problem hiding this comment.
What about the other combinations? Would it be possible (but not too cumbersome) to parameterize this by the two build types, and whether takeover is allowed?
2 participants
The
CONTINUEpath inOnlineIndexer.indexingCatchercould re-apply an adjustment that was already in effect. An unresolvable "This index was partly built by another method" mismatch was therefore retried identically until reaching max attempts, ending with a misleading "Too many indexing attempts" error.