feat(mango): add keys_examined for execution_stats
#4569
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
pgj
force-pushed
the
feat/mango-execution-stats-keys-examined
branch
2 times, most recently
from
98d47bc to
92febd4
Compare
nickva
reviewed
May 5, 2023
nickva
reviewed
May 5, 2023
nickva
approved these changes
May 5, 2023
pgj
force-pushed
the
feat/mango-execution-stats-keys-examined
branch
2 times, most recently
from
604beef to
20eabcb
Compare
|
Please do not merge this now, I would like to do some refactoring (move the process dictionary function under |
pgj
force-pushed
the
feat/mango-execution-stats-keys-examined
branch
2 times, most recently
from
2e0d2f9 to
583e396
Compare
|
OK -- the planned changes are there. |
chewbranca
reviewed
May 10, 2023
There was a problem hiding this comment.
Awesome, @pgj! Thanks for moving the process dictionary manipulation to a dedicated module. I left a minor nit on assigning variables within a case statement, but otherwise looks good to me.
pgj
force-pushed
the
feat/mango-execution-stats-keys-examined
branch
from
583e396 to
3ecd6a1
Compare
chewbranca
approved these changes
May 10, 2023
nickva
reviewed
May 10, 2023
pgj
force-pushed
the
feat/mango-execution-stats-keys-examined
branch
from
3ecd6a1 to
c39bd1d
Compare
Add another field to the shard-level Mango execution statistics to keep track of the count of keys that were examined for the query. Note that this requires to change the way how stats are stored -- an approach similar to that of the view callback arguments was chosen, which features a map. This current version supports both the old and new formats. The coordinator may request getting the results in the new one by adding `execution_stats_map` for the arguments of the view callback. Otherwise the old format is used (without the extra field), which makes it possible to work with older coordinators. Old workers will automatically ignore this argument and answer in the old format.
pgj
force-pushed
the
feat/mango-execution-stats-keys-examined
branch
from
c39bd1d to
2351468
Compare
2 tasks
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.
Add another field to the shard-level Mango execution statistics to keep track of the count of keys that were examined for the query. Note that this requires to change the way how stats are stored -- an approach similar to that of the view callback arguments was chosen, which features a map (#4394).
The change in the format also implies a potential incompatibility between different version of nodes, which may happen for mixed-version clusters. This is handled by introducing an additional option for the view callback where the coordinator can request for the results in the new format and the old format is still recognized. It helps to handle version differences automatically therefore no specific configuration option is required to control the migration. Thanks @rnewson for the idea.
This is related to the covering indexes work and takes inspiration from earlier works of @mikerhodes , see #4410 and #4413 for the details.
Note that slight increase in unit test coverage for
mango_execution_statsdue to the implicit testing viamango_cursor_view. The only missing part isincr_docs_examined/1but that is not used from there, only frommango_cursor_textandmango_cursor_nouveau.$ make eunit apps=mangobefore:
after:
Checklist