Skip to content

fix(shell-api): sharding methods account for 5.0 config.chunks change… #834

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
May 3, 2021
Merged

fix(shell-api): sharding methods account for 5.0 config.chunks change… #834

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
625b798
fix(shell-api): sharding methods account for 5.0 config.chunks change…
addaleax May 3, 2021
8b3faf3
fixup! fix(shell-api): sharding methods account for 5.0 config.chunks…
addaleax May 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/shell-api/src/collection.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1622,6 +1622,10 @@ describe('Collection', () => {
});
describe('getShardDistribution', () => {
it('throws when collection is not sharded', async() => {
const serviceProviderCursor = stubInterface<ServiceProviderCursor>();
serviceProviderCursor.limit.returns(serviceProviderCursor);
serviceProviderCursor.tryNext.returns(null);
serviceProvider.find.returns(serviceProviderCursor as any);
const error = await collection.getShardDistribution().catch(e => e);

expect(error).to.be.instanceOf(MongoshInvalidInputError);
Expand Down
20 changes: 14 additions & 6 deletions packages/shell-api/src/collection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1530,21 +1530,23 @@ export default class Collection extends ShellApiClass {

const result = {} as Document;
const config = this._mongo.getDB('config');
const ns = `${this._database._name}.${this._name}`;

const isSharded = !!(await config.getCollection('collections').countDocuments({
_id: `${this._database._name}.${this._name}`,
const configCollectionsInfo = await config.getCollection('collections').findOne({
_id: ns,
// dropped is gone on newer server versions, so check for !== true
// rather than for === false (SERVER-51880 and related)
dropped: { $ne: true }
}));
if (!isSharded) {
});
if (!configCollectionsInfo) {
throw new MongoshInvalidInputError(
`Collection ${this._name} is not sharded`,
ShellApiErrors.NotConnectedToShardedCluster
);
}

const collStats = await (await this.aggregate({ '$collStats': { storageStats: {} } })).toArray();
const uuid = configCollectionsInfo?.uuid ?? null;

const totals = { numChunks: 0, size: 0, count: 0 };
const conciseShardsStats: {
Expand All @@ -1559,12 +1561,18 @@ export default class Collection extends ShellApiClass {
await Promise.all(collStats.map((extShardStats) => (
(async(): Promise<void> => {
// Extract and store only the relevant subset of the stats for this shard
const { shard } = extShardStats;

// If we have an UUID, use that for lookups. If we have only the ns,
// use that. (On 5.0+ servers, config.chunk has uses the UUID, before
// that it had the ns).
const countChunksQuery = uuid ? { $or: [ { uuid }, { ns } ], shard } : { ns, shard };
const [ host, numChunks ] = await Promise.all([
config.getCollection('shards').findOne({ _id: extShardStats.shard }),
config.getCollection('chunks').countDocuments({ ns: extShardStats.ns, shard: extShardStats.shard })
config.getCollection('chunks').countDocuments(countChunksQuery)
]);
const shardStats = {
shardId: extShardStats.shard,
shardId: shard,
host: host !== null ? host.host : null,
size: extShardStats.storageStats.size,
count: extShardStats.storageStats.count,
Expand Down
17 changes: 10 additions & 7 deletions packages/shell-api/src/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,8 +403,8 @@ export async function getPrintableShardStatus(db: Database, verbose: boolean): P

// Special case the config db, since it doesn't have a record in config.databases.
databases.push({ '_id': 'config', 'primary': 'config', 'partitioned': true });
databases.sort((a: any, b: any): any => {
return a._id > b._id;
databases.sort((a: any, b: any): number => {
return a._id.localeCompare(b._id);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just something I noticed, this would obviously not have worked before because the .sort() callback should not return a boolean.

});

result.databases = await Promise.all(databases.map(async(db) => {
Expand All @@ -429,12 +429,15 @@ export async function getPrintableShardStatus(db: Database, verbose: boolean): P
collRes.balancing = [ !coll.noBalance, { noBalance: coll.noBalance } ];
}
const chunksRes = [];
const chunksCollMatch =
coll.uuid ? { $or: [ { uuid: coll.uuid }, { ns: coll._id } ] } : { ns: coll._id };
const chunks = await
(await chunksColl.aggregate({ $match: { ns: coll._id } },
(await chunksColl.aggregate([
{ $match: chunksCollMatch },
{ $group: { _id: '$shard', cnt: { $sum: 1 } } },
{ $project: { _id: 0, shard: '$_id', nChunks: '$cnt' } },
{ $sort: { shard: 1 } })
).toArray();
{ $sort: { shard: 1 } }
])).toArray();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The tests really should have discovered that sh.status() was also broken, I just noticed this because I was looking for other uses of config.chunks in the shell-api source. I’ve opened https://jira.mongodb.org/browse/MONGOSH-742 as a reminder to write proper tests here.

let totalChunks = 0;
chunks.forEach((z: any) => {
totalChunks += z.nChunks;
Expand All @@ -443,7 +446,7 @@ export async function getPrintableShardStatus(db: Database, verbose: boolean): P

// NOTE: this will return the chunk info as a string, and will print ugly BSON
if (totalChunks < 20 || verbose) {
(await chunksColl.find({ 'ns': coll._id })
(await chunksColl.find(chunksCollMatch)
.sort({ min: 1 }).toArray())
.forEach((chunk: any) => {
const c = {
Expand All @@ -461,7 +464,7 @@ export async function getPrintableShardStatus(db: Database, verbose: boolean): P

const tagsRes: any[] = [];
(await configDB.getCollection('tags')
.find({ ns: coll._id })
.find(chunksCollMatch)
.sort({ min: 1 })
.toArray())
.forEach((tag: any) => {
Expand Down
14 changes: 7 additions & 7 deletions packages/shell-api/src/shard.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1159,9 +1159,9 @@ describe('Shard', () => {
expect((await sh.status()).value.databases.length).to.equal(2);
});
it('enableSharding for a collection and modify documents in it', async() => {
expect(Object.keys((await sh.status()).value.databases[0].collections).length).to.equal(0);
expect(Object.keys((await sh.status()).value.databases[1].collections).length).to.equal(0);
expect((await sh.shardCollection(ns, { key: 1 })).collectionsharded).to.equal(ns);
expect((await sh.status()).value.databases[0].collections[ns].shardKey).to.deep.equal({ key: 1 });
expect((await sh.status()).value.databases[1].collections[ns].shardKey).to.deep.equal({ key: 1 });

const db = internalState.currentDb.getSiblingDB(dbName);
await db.coll.insertMany([{ key: 'A', value: 10 }, { key: 'B', value: 20 }]);
Expand Down Expand Up @@ -1189,20 +1189,20 @@ describe('Shard', () => {
});
it('sets a zone key range', async() => {
expect((await sh.updateZoneKeyRange(ns, { key: 0 }, { key: 20 }, 'zone1')).ok).to.equal(1);
expect((await sh.status()).value.databases[0].collections[ns].tags[0]).to.deep.equal({
expect((await sh.status()).value.databases[1].collections[ns].tags[0]).to.deep.equal({
tag: 'zone1', min: { key: 0 }, max: { key: 20 }
});
expect((await sh.addTagRange(ns, { key: 21 }, { key: 40 }, 'zone0')).ok).to.equal(1);
expect((await sh.status()).value.databases[0].collections[ns].tags[1]).to.deep.equal({
expect((await sh.status()).value.databases[1].collections[ns].tags[1]).to.deep.equal({
tag: 'zone0', min: { key: 21 }, max: { key: 40 }
});
});
it('removes a key range', async() => {
expect((await sh.status()).value.databases[0].collections[ns].tags.length).to.equal(2);
expect((await sh.status()).value.databases[1].collections[ns].tags.length).to.equal(2);
expect((await sh.removeRangeFromZone(ns, { key: 0 }, { key: 20 })).ok).to.equal(1);
expect((await sh.status()).value.databases[0].collections[ns].tags.length).to.equal(1);
expect((await sh.status()).value.databases[1].collections[ns].tags.length).to.equal(1);
expect((await sh.removeTagRange(ns, { key: 21 }, { key: 40 })).ok).to.equal(1);
expect((await sh.status()).value.databases[0].collections[ns].tags.length).to.equal(0);
expect((await sh.status()).value.databases[1].collections[ns].tags.length).to.equal(0);
});
it('removes zones', async() => {
expect((await sh.removeShardFromZone(`${shardId}-1`, 'zone1')).ok).to.equal(1);
Expand Down