add GetMemoryDomainsBySocket and TopologyMaps #364
Conversation
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
add memoryDomain to graph query and fix typo for scope granularity
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.
When adding the
numastatscollector to our instance, we encountered the following error:METRICDATA/CCMS > TODO: unhandled case: native-scope=memoryDomain, requested-scope=socket. To address this, we added theGetMemoryDomainsBySocketfunction tocluster.go, which resolves the issue.While reviewing
cluster.go, we noticed triple-nested loops accompanied by comments such asTODO: Optimize this, there must be a more efficient way/algorithm. This PR introduces that optimization.We implemented a topology mapping for each cluster and subcluster, which is initialized and cached in memory. This allows
buildQueriesandbuildNodeQueriesto perform simple lookups instead of iterating over every HWThread. After running this change in our environment for one week, we observed no error messages in the logs. However, we’d appreciate it if the maintainers could review this to ensure we haven’t inadvertently introduced any issues.