Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(shell-api): use collStats aggregation stage instead of command for collection.stats() MONGOSH-1157 #1384

Merged
merged 10 commits into from
Jan 18, 2023

Conversation

Anemy
Copy link
Member

@Anemy Anemy commented Jan 11, 2023

MONGOSH-1157

This PR updates our db.collection.stats() shell api method to use the aggregation stage $collStats for fetching collection statistics instead of the db.runCommand({ collStats: collectionName }) form. The run command form is to be deprecated by 7.0 (PM-2362)

The $collStats stage returns a document for each individual shard's statistics. A lot of the code of this pr involves aggregating those documents into one document to be returned. I initially was trying to do this all in one aggregation pipeline, however I found it was going to be a bit harder to test and maintain. Things like new keys like numOrphanDocs added in 6.0 make an aggregation a lot more complex. If curious a fairly non-optimized implementation (without timeseries support) can be found on this gist: https://gist.github.com/Anemy/1e3b38f8f4f07f055a8c18d4b6ab443b

The $collStats aggregation stage fails on sharded timeseries collections so we fallback to the old command form when that error code is encountered.

Notes

  • These changes alter the output format of db.collection.stats()
    Using the aggregation $collStats stage does not return the same data
    as the soon to be deprecated collStats command. Namely:
    - primary is not given now.
    - nchunks is not given now.
    These fields already aren't listed in our documentation: https://www.mongodb.com/docs/manual/reference/command/collStats/ but I'm curious to hear if folks feel we should work to re-add them or have more insights.

Copy link
Contributor

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the way we're checking for sharding okay? Is there a better way? Does the current way require special permissions?

Yeah, I think it does require permissions that not all users might have… we could maybe fall back to doing something like the (somewhat inaccurate) collStats.length > 1 if we cannot query config.collections?

packages/shell-api/src/collection.ts Outdated Show resolved Hide resolved
packages/shell-api/src/collection.ts Outdated Show resolved Hide resolved
packages/shell-api/src/collection.ts Outdated Show resolved Hide resolved
@Anemy Anemy marked this pull request as ready for review January 12, 2023 18:24
@addaleax addaleax merged commit 98cc3e5 into main Jan 18, 2023
@addaleax addaleax deleted the MONGOSH-1157-aggregate-collstats-programmatically branch January 18, 2023 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants