state: remove duplicate tableCheck indexes #9796
Merged
Conversation
This file contains 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
catalogChecksForNodeService was a duplicate of catalogListServiceChecks
By using a new pattern for more specific indexes. This allows us to use the same index for both service checks and node checks. It removes the abstraction around memdb.Txn operations, and isolates all of the enterprise differences in a single place (the indexer).
These new functional indexers provide a few advantages: 1. enterprise differences can be isolated to a single function (the indexer function), making code easier to change 2. as a consequence of (1) we no longer need to wrap all the calls to Txn operations, making code easier to read. 3. by removing reflection we should increase the performance of all operations. One important change is in making all the function signatures the same. https://blog.golang.org/errors-are-values An extra boolean return value for SingleIndexer.FromObject is superfluous. The error value can indicate when the index value could not be created. By removing this extra return value we can use the same signature for both indexer functions. This has the nice properly of a function being usable for both indexing operations.
|
🤔 Double check that this PR does not require a changelog entry in the |
dnephin
added
the
pr/no-changelog
|
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/335707. |
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.
This PR uses a new pattern (see
agent/consul/state/indexer.go) for defining memdb indexes in the state store.By using this pattern we are able to remove one of the indexes on the check table. The generic index did not behave the way we wanted, which forced us to create a second index. Now we can use a single index because we add the blank string value as part of the index.
Since each section of the index is null terminated, there is no ambiguity. Node checks will have two sequential null bytes at the end, but that is ok.
This PR also makes the new function signature for indexFromObject match that of indexFromQuery by using errors as values. This will allow us to use the same function for both sets of operations (queries and insert/delete).
It also adds an indexBuilder which can be used instead of a bytes.Buffer to add null terminators, and makes the index building a little less verbose.