-
Notifications
You must be signed in to change notification settings - Fork 67
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): add shardedDataDistribution to sh.status() MONGOSH-1326 #2214
Conversation
'Time of Reported error'?: string; | ||
'Collections with active migrations'?: `${string} started at ${string}`[]; | ||
}; | ||
shardedDataDistribution?: ShardedDataDistribution; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am adding it as a separate field at the moment since that's just a simple solution, but we could make it cleaner/nicer for the user. Open to suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I'm no expert in this department, though.
…ongosh into gagik/status-6.0.3-clusters
packages/shell-api/src/helpers.ts
Outdated
): Promise<ShardingStatusResult> { | ||
const result = {} as ShardingStatusResult; | ||
|
||
const configDB = await getConfigDB(db); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh... this is an unintentional breaking change, no? You can't pass a configDB
object to sh.status()
explicitly anymore after this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, my bad, somehow I thought the getConfigDB
was always happening already.
I suppose if configDB
is passed explicitly then we won't end up doing shardedDataDistribution? or does getSiblingDb
imply one can get admin
db from a given config
db?
EDIT: Okay yeah I see, so it doesn't matter that the DB is root. I assume it also didn't break much because if I understand correctly a configDB could still get configDB as its sibling but this is unintentional and I'll remove that
Co-authored-by: Anna Henningsen <[email protected]>
Co-authored-by: Anna Henningsen <[email protected]>
migrations.forEach((x: any) => { | ||
const migrationsRes: ShardingStatusResult['balancer']['Migration Results for the last 24 hours'] = | ||
{}; | ||
migrations.forEach((x) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note for the future, but if you run into existing .forEach()
into the future, don't be shy to refactor them to for ... of
🙂
* main: chore: update auto-generated files (#2237) feat(shell-api): add shardedDataDistribution to sh.status() MONGOSH-1326 (#2214) chore: update auto-generated files (#2235) feat(tests): Add individual evergreen test results with XUnit (#2227) chore: update auto-generated files (#2234) chore: rename service-provider-server and `CliServiceProvider` (#2232) chore: update auto-generated files (#2233) chore(deps): Add node-gyp version control across different distros MONGOSH-1891 (#2230) chore: update auto-generated files (#2231) fix(shell-api): Align database and collection aggregate functions MONGOSH-1868 (#2229)
Considering the balancing of a sharded namespace would be sensitive to orphans and data volume, and in order to count with a mechanism that easily allows checking the current state of the cluster, it would be good if the sh.status() provides the following information per sharded namespaces:
This is supported in
6.0.3+