Skip no-op delta rebuilds for root-local hash indexes#805
Open
tnabulsi-netflix wants to merge 1 commit intoNetflix:masterfrom
Open
Skip no-op delta rebuilds for root-local hash indexes#805tnabulsi-netflix wants to merge 1 commit intoNetflix:masterfrom
tnabulsi-netflix wants to merge 1 commit intoNetflix:masterfrom
Conversation
3b230ac to
578f002
Compare
| this.hashStateVolatile = new HollowHashIndexState(builder); | ||
| } | ||
|
|
||
| private boolean canSkipNoOpDeltaUpdate() { |
Collaborator
There was a problem hiding this comment.
This part is not needed, as ordinal change is a direct signal on whether or not a hash needs to rebuilt.
One reason is that, for top level type A, if its member type B has a field change, the change would cause the type A record to have an ordinal change as well.
This statement holds true, also because:
- for a type, an ordinal will not be reassigned within a single cycle.
- Hollow Data set version update via delta would apply every delta on the version chain, and would not skip a version.
- a sub type's ordinal change would propagate all the way up to the top level type, all the record along the hierarchy would get assigned a new ordinal.
But also wonder if there are other edge cases..
Collaborator
There was a problem hiding this comment.
@azeng-netflix need to check if double snapshot scenario is also handled and add unit test that would check this invariant in the future.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
HollowHashIndexdelta reindexing when the index is root-select and all match fields are direct non-reference fields on the root typeTesting
./gradlew :hollow:test --tests com.netflix.hollow.core.index.HollowHashIndexTest --rerun-tasks -Pcom.netflix.gradle-predictive-test-selection.enabled=false