diff --git a/packages/compass-crud/src/utils/cancellable-queries.spec.ts b/packages/compass-crud/src/utils/cancellable-queries.spec.ts index ef7530cef12..8079a0e0149 100644 --- a/packages/compass-crud/src/utils/cancellable-queries.spec.ts +++ b/packages/compass-crud/src/utils/cancellable-queries.spec.ts @@ -18,8 +18,8 @@ describe('cancellable-queries', function () { const cluster = mochaTestServer(); let dataService: DataService; let preferences: PreferencesAccess; - let abortController; - let signal; + let abortController: AbortController; + let signal: AbortSignal; before(async function () { preferences = await createSandboxFromDefaultPreferences(); @@ -99,7 +99,7 @@ describe('cancellable-queries', function () { dataService, preferences, 'cancel.numbers', - null, + {}, { signal, } @@ -138,7 +138,7 @@ describe('cancellable-queries', function () { dataService, preferences, 'cancel.numbers', - 'this is not a filter', + {}, { signal, hint: { _id_: 1 }, // this collection doesn't have this index so this query should fail diff --git a/packages/compass-crud/src/utils/cancellable-queries.ts b/packages/compass-crud/src/utils/cancellable-queries.ts index 8e477c80983..b748cabddb1 100644 --- a/packages/compass-crud/src/utils/cancellable-queries.ts +++ b/packages/compass-crud/src/utils/cancellable-queries.ts @@ -79,19 +79,13 @@ export async function fetchShardingKeys( } ): Promise { try { - const docs = await dataService.find( - 'config.collections', - { - _id: ns as any, - // unsplittable introduced in SPM-3364 to mark unsharded collections - // that are still being tracked in the catalog - unsplittable: { $ne: true }, - }, - { maxTimeMS, projection: { key: 1, _id: 0 } }, + const shardKey = await dataService.fetchShardKey( + ns, + { maxTimeMS }, { abortSignal: signal } ); - return docs.length ? docs[0].key : {}; - } catch (err: any) { + return shardKey ?? {}; + } catch (err) { // rethrow if we aborted along the way if (dataService.isCancelError(err)) { throw err; diff --git a/packages/compass-crud/src/utils/data-service.ts b/packages/compass-crud/src/utils/data-service.ts index 6f398a0363d..e9351819569 100644 --- a/packages/compass-crud/src/utils/data-service.ts +++ b/packages/compass-crud/src/utils/data-service.ts @@ -21,7 +21,8 @@ export type RequiredDataServiceProps = | 'collectionStats' | 'collectionInfo' | 'listCollections' - | 'isListSearchIndexesSupported'; + | 'isListSearchIndexesSupported' + | 'fetchShardKey'; // TODO: It might make sense to refactor the DataService interface to be closer to // { ..., getCSFLEMode(): 'unavailable' } | { ..., getCSFLEMode(): 'unavailable' | 'enabled' | 'disabled', isUpdateAllowed(): ..., knownSchemaForCollection(): ... } // so that either these methods are always present together or always absent diff --git a/packages/compass-indexes/src/components/regular-indexes-table/property-field.tsx b/packages/compass-indexes/src/components/regular-indexes-table/property-field.tsx index 9c36e1ba7ad..377cd557f4d 100644 --- a/packages/compass-indexes/src/components/regular-indexes-table/property-field.tsx +++ b/packages/compass-indexes/src/components/regular-indexes-table/property-field.tsx @@ -35,6 +35,19 @@ export const getPropertyTooltip = ( return null; }; +const HIDDEN_INDEX_TEXT = 'HIDDEN'; +const SHARD_KEY_INDEX_TEXT = 'SHARD KEY'; + +export const getPropertyText = ( + property: RegularIndex['properties'][number] +): string => { + if (property === 'shardKey') { + return SHARD_KEY_INDEX_TEXT; + } + + return property; +}; + const PropertyBadgeWithTooltip: React.FunctionComponent<{ text: string; link: string; @@ -65,8 +78,6 @@ type PropertyFieldProps = { properties: RegularIndex['properties']; }; -const HIDDEN_INDEX_TEXT = 'HIDDEN'; - const PropertyField: React.FunctionComponent = ({ extra, properties, @@ -79,7 +90,7 @@ const PropertyField: React.FunctionComponent = ({ return ( diff --git a/packages/compass-indexes/src/utils/index-link-helper.ts b/packages/compass-indexes/src/utils/index-link-helper.ts index 50c896dce7c..1a2af9f3a58 100644 --- a/packages/compass-indexes/src/utils/index-link-helper.ts +++ b/packages/compass-indexes/src/utils/index-link-helper.ts @@ -22,10 +22,12 @@ const HELP_URLS = { 'https://docs.mongodb.com/master/reference/bson-type-comparison-order/#collation', COLLATION_REF: 'https://docs.mongodb.com/master/reference/collation', HIDDEN: 'https://www.mongodb.com/docs/manual/core/index-hidden/', + SHARDKEY: 'https://www.mongodb.com/docs/manual/core/sharding-shard-key/', UNKNOWN: null, }; export type HELP_URL_KEY = + | 'shardKey' // The only camelCase key at the moment. | Uppercase | Lowercase; diff --git a/packages/data-service/src/data-service.spec.ts b/packages/data-service/src/data-service.spec.ts index 2d63fb4cfd7..5301cb47e90 100644 --- a/packages/data-service/src/data-service.spec.ts +++ b/packages/data-service/src/data-service.spec.ts @@ -115,7 +115,7 @@ describe('DataService', function () { fatal: () => {}, }; - let dataServiceLogTest; + let dataServiceLogTest: DataService | undefined; beforeEach(function () { logs.length = 0; @@ -1557,6 +1557,24 @@ describe('DataService', function () { }); }); + describe('#fetchShardKey', function () { + beforeEach(async function () { + await mongoClient + .db(testDatabaseName) + .collection(testCollectionName) + .createIndex( + { + a: 1, + }, + {} + ); + }); + + it('fetches the shard key (there is none in this test)', async function () { + expect(await dataService.fetchShardKey(testNamespace)).to.equal(null); + }); + }); + describe('CSFLE logging', function () { it('picks a selected set of CSFLE options for logging', function () { const fleOptions: ConnectionFleOptions = { @@ -2010,6 +2028,122 @@ describe('DataService', function () { }); }); + context('with real sharded cluster', function () { + this.slow(10_000); + this.timeout(20_000); + + const cluster = mochaTestServer({ + topology: 'sharded', + secondaries: 0, + }); + + let dataService: DataServiceImpl; + let mongoClient: MongoClient; + let connectionOptions: ConnectionOptions; + let testCollectionName: string; + let testDatabaseName: string; + let testNamespace: string; + + before(async function () { + testDatabaseName = `compass-data-service-sharded-tests`; + const connectionString = cluster().connectionString; + connectionOptions = { + connectionString, + }; + + mongoClient = new MongoClient(connectionOptions.connectionString); + await mongoClient.connect(); + + dataService = new DataServiceImpl(connectionOptions); + await dataService.connect(); + }); + + after(async function () { + // eslint-disable-next-line no-console + await dataService?.disconnect().catch(console.log); + await mongoClient?.close(); + }); + + beforeEach(async function () { + testCollectionName = `coll-${new UUID().toString()}`; + testNamespace = `${testDatabaseName}.${testCollectionName}`; + + await mongoClient + .db(testDatabaseName) + .collection(testCollectionName) + .insertMany(TEST_DOCS); + + await mongoClient + .db(testDatabaseName) + .collection(testCollectionName) + .createIndex( + { + a: 1, + }, + {} + ); + }); + + afterEach(async function () { + sinon.restore(); + + await mongoClient + .db(testDatabaseName) + .collection(testCollectionName) + .drop(); + }); + + describe('with a sharded collection', function () { + beforeEach(async function () { + await runCommand(dataService['_database']('admin', 'META'), { + shardCollection: testNamespace, + key: { + a: 1, + }, + // We don't run the shardCollection command outside of tests + // so it isn't part of the runCommand type. + } as unknown as Parameters[1]); + }); + + describe('#fetchShardKey', function () { + it('fetches the shard key', async function () { + expect(await dataService.fetchShardKey(testNamespace)).to.deep.equal({ + a: 1, + }); + + // Can be cancelled. + const abortController = new AbortController(); + const abortSignal = abortController.signal; + const promise = dataService + .fetchShardKey( + testNamespace, + {}, + { abortSignal: abortSignal as unknown as AbortSignal } + ) + .catch((err) => err); + abortController.abort(); + const error = await promise; + + expect(dataService.isCancelError(error)).to.be.true; + }); + }); + + describe('#indexes', function () { + it('includes the shard key', async function () { + const indexes = await dataService.indexes(testNamespace); + + expect(indexes.length).to.equal(2); + expect( + indexes.find((index) => index.key._id === 1)?.properties + ).to.not.include('shardKey'); + expect( + indexes.find((index) => index.key.a === 1)?.properties + ).to.include('shardKey'); + }); + }); + }); + }); + context('with mocked client', function () { function createDataServiceWithMockedClient( clientConfig: Partial diff --git a/packages/data-service/src/data-service.ts b/packages/data-service/src/data-service.ts index 83d30bc9287..a7cadd71fba 100644 --- a/packages/data-service/src/data-service.ts +++ b/packages/data-service/src/data-service.ts @@ -733,6 +733,19 @@ export interface DataService { } ): Promise; + /** + * Fetch shard keys for the collection from the collections config. + * + * @param ns - The namespace to try to find shard key for. + * @param options - The query options. + * @param executionOptions - The execution options. + */ + fetchShardKey( + ns: string, + options?: Omit, + executionOptions?: ExecutionOptions + ): Promise | null>; + /*** Insert ***/ /** @@ -2234,6 +2247,44 @@ class DataServiceImpl extends WithLogContext implements DataService { return indexToProgress; } + @op(mongoLogId(1_001_000_380)) + async fetchShardKey( + ns: string, + options: Omit = {}, + executionOptions?: ExecutionOptions + ): Promise | null> { + const docs = await this.find( + 'config.collections', + { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + _id: ns as any, + // unsplittable introduced in SPM-3364 to mark unsharded collections + // that are still being tracked in the catalog + unsplittable: { $ne: true }, + }, + { ...options, projection: { key: 1, _id: 0 } }, + { abortSignal: executionOptions?.abortSignal } + ); + return docs.length ? docs[0].key : null; + } + + private async _fetchShardKeyWithSilentFail( + ...args: Parameters + ): ReturnType { + try { + return await this.fetchShardKey(...args); + } catch (err) { + // Rethrow if we aborted along the way. + if (this.isCancelError(err)) { + throw err; + } + + // Return null on error. + // This is oftentimes a lack of permissions to run a find on config.collections. + return null; + } + } + @op(mongoLogId(1_001_000_047)) async indexes( ns: string, @@ -2245,19 +2296,21 @@ class DataServiceImpl extends WithLogContext implements DataService { ); return indexes.map((compactIndexEntry) => { const [name, keys] = compactIndexEntry; - return createIndexDefinition(ns, { + return createIndexDefinition(ns, null, { name, key: Object.fromEntries(keys), }); }); } - const [indexes, indexStats, indexSizes, indexProgress] = await Promise.all([ - this._collection(ns, 'CRUD').indexes({ ...options, full: true }), - this._indexStats(ns), - this._indexSizes(ns), - this._indexProgress(ns), - ]); + const [indexes, indexStats, indexSizes, indexProgress, shardKey] = + await Promise.all([ + this._collection(ns, 'CRUD').indexes({ ...options, full: true }), + this._indexStats(ns), + this._indexSizes(ns), + this._indexProgress(ns), + this._fetchShardKeyWithSilentFail(ns), + ]); const maxSize = Math.max(...Object.values(indexSizes)); @@ -2269,6 +2322,7 @@ class DataServiceImpl extends WithLogContext implements DataService { const name = index.name; return createIndexDefinition( ns, + shardKey ?? null, index, indexStats[name], indexSizes[name], diff --git a/packages/data-service/src/index-detail-helper.spec.ts b/packages/data-service/src/index-detail-helper.spec.ts index 49b837b7f3a..6476452860d 100644 --- a/packages/data-service/src/index-detail-helper.spec.ts +++ b/packages/data-service/src/index-detail-helper.spec.ts @@ -1,8 +1,13 @@ import { expect } from 'chai'; -import type { IndexInfo } from './index-detail-helper'; +import type { IndexDescriptionInfo, IndexDirection } from 'mongodb'; + import { createIndexDefinition } from './index-detail-helper'; -const SIMPLE_INDEX: IndexInfo = { +type IndexDescriptionWithName = IndexDescriptionInfo & { + name: string; +}; + +const SIMPLE_INDEX: IndexDescriptionWithName = { v: 1, key: { foo: 1, @@ -10,7 +15,7 @@ const SIMPLE_INDEX: IndexInfo = { name: 'foo', }; -const INDEXES_FIXTURE: IndexInfo[] = [ +const INDEXES_FIXTURE: IndexDescriptionWithName[] = [ { v: 1, key: { @@ -129,7 +134,7 @@ const INDEXES_FIXTURE: IndexInfo[] = [ { v: 1, key: { - '$**': 'columnstore', + '$**': 'columnstore' as IndexDirection /* Coming in a later version. */, }, name: 'columnstore_single_subtree', ns: 'mongodb.fanclub', @@ -137,7 +142,16 @@ const INDEXES_FIXTURE: IndexInfo[] = [ { v: 1, key: { - 'address.$**': 'columnstore', + aShardedField: 1, + }, + name: 'shardedIndexKey', + ns: 'mongodb.fanclub', + }, + { + v: 1, + key: { + 'address.$**': + 'columnstore' as IndexDirection /* Coming in a later version. */, }, name: 'columnstore_multi_subtree', columnstoreProjection: { @@ -150,7 +164,13 @@ const INDEXES_FIXTURE: IndexInfo[] = [ describe('createIndexDefinition', function () { const definitions = INDEXES_FIXTURE.map((index) => { - return createIndexDefinition('mongodb.fanclub', index); + return createIndexDefinition( + 'mongodb.fanclub', + { + aShardedField: 1, + }, + index + ); }); const definitionsMap = new Map( definitions.map((index) => [index.name, index]) @@ -170,6 +190,7 @@ describe('createIndexDefinition', function () { 'not_wildcard', 'seniors', 'seniors-inverse', + 'shardedIndexKey', 'wildcard_multi_subtree', 'wildcard_single_subtree', ]); @@ -269,6 +290,23 @@ describe('createIndexDefinition', function () { ); }); + it('should recognize shard keys', function () { + expect( + definitions.every( + (index) => + index.name === 'shardedIndexKey' || + !index.properties.includes('shardKey') + ) + ).to.eq( + true, + 'expected every index to not have the shard key excluding shardedIndexKey' + ); + + expect(definitionsMap.get('shardedIndexKey')) + .to.have.property('properties') + .include('shardKey'); + }); + describe('cardinality', function () { it('non-text simple index', function () { const index = { @@ -279,7 +317,7 @@ describe('createIndexDefinition', function () { name: 'age_index', ns: 'mongodb.fanclub', }; - expect(createIndexDefinition('', index)).to.have.property( + expect(createIndexDefinition('', null, index)).to.have.property( 'cardinality', 'single' ); @@ -295,14 +333,14 @@ describe('createIndexDefinition', function () { name: 'age_index', ns: 'mongodb.fanclub', }; - expect(createIndexDefinition('', index)).to.have.property( + expect(createIndexDefinition('', null, index)).to.have.property( 'cardinality', 'compound' ); }); it('simple text index', function () { - const index = { + const index: IndexDescriptionWithName = { v: 1, key: { _fts: 'text', @@ -317,14 +355,14 @@ describe('createIndexDefinition', function () { language_override: 'language', textIndexVersion: 3, }; - expect(createIndexDefinition('', index)).to.have.property( + expect(createIndexDefinition('', null, index)).to.have.property( 'cardinality', 'single' ); }); it('text index on multiple fields which are all text', function () { - const index = { + const index: IndexDescriptionWithName = { v: 1, key: { _fts: 'text', @@ -340,14 +378,14 @@ describe('createIndexDefinition', function () { language_override: 'language', textIndexVersion: 3, }; - expect(createIndexDefinition('', index)).to.have.property( + expect(createIndexDefinition('', null, index)).to.have.property( 'cardinality', 'compound' ); }); it('text index on multiple fields which are mixed', function () { - const index = { + const index: IndexDescriptionWithName = { v: 1, key: { _fts: 'text', @@ -363,7 +401,7 @@ describe('createIndexDefinition', function () { language_override: 'language', textIndexVersion: 3, }; - expect(createIndexDefinition('', index)).to.have.property( + expect(createIndexDefinition('', null, index)).to.have.property( 'cardinality', 'compound' ); @@ -372,19 +410,25 @@ describe('createIndexDefinition', function () { it('calculates correct ttl', function () { expect( - createIndexDefinition('', { ...SIMPLE_INDEX, expireAfterSeconds: 20 }) + createIndexDefinition('', null, { + ...SIMPLE_INDEX, + expireAfterSeconds: 20, + }) ) .to.have.property('properties') .include('ttl'); expect( - createIndexDefinition('', { ...SIMPLE_INDEX, expireAfterSeconds: 0 }) + createIndexDefinition('', null, { + ...SIMPLE_INDEX, + expireAfterSeconds: 0, + }) ) .to.have.property('properties') .include('ttl'); expect( - createIndexDefinition('', { + createIndexDefinition('', null, { ...SIMPLE_INDEX, expireAfterSeconds: undefined, }) @@ -392,7 +436,7 @@ describe('createIndexDefinition', function () { .to.have.property('properties') .not.include('ttl'); - expect(createIndexDefinition('', { ...SIMPLE_INDEX })) + expect(createIndexDefinition('', null, { ...SIMPLE_INDEX })) .to.have.property('properties') .not.include('ttl'); }); diff --git a/packages/data-service/src/index-detail-helper.ts b/packages/data-service/src/index-detail-helper.ts index 2219562363a..72c32357c7a 100644 --- a/packages/data-service/src/index-detail-helper.ts +++ b/packages/data-service/src/index-detail-helper.ts @@ -1,3 +1,5 @@ +import { isDeepStrictEqual } from 'util'; +import { EJSON } from 'bson'; import type { IndexDescriptionInfo } from 'mongodb'; export type IndexInfo = { @@ -33,7 +35,14 @@ export type IndexDefinition = { | 'clustered' | 'columnstore'; cardinality: 'single' | 'compound'; - properties: ('unique' | 'sparse' | 'partial' | 'ttl' | 'collation')[]; + properties: ( + | 'unique' + | 'sparse' + | 'partial' + | 'ttl' + | 'collation' + | 'shardKey' + )[]; extra: Record>; size: IndexSize; relativeSize: number; @@ -56,7 +65,8 @@ export function getIndexCardinality( } export function getIndexProperties( - index: Pick + index: Pick, + collectionShardKey: unknown ): IndexDefinition['properties'] { const properties: IndexDefinition['properties'] = []; @@ -80,6 +90,16 @@ export function getIndexProperties( properties.push('collation'); } + if ( + collectionShardKey && + isDeepStrictEqual( + EJSON.serialize(collectionShardKey), + EJSON.serialize(index.key) + ) + ) { + properties.push('shardKey'); + } + return properties; } @@ -120,7 +140,15 @@ export function getIndexType( export function createIndexDefinition( ns: string, - { name, key, v, ...extra }: IndexDescriptionInfo & { name: string }, + collectionShardKey: unknown, + { + name, + key, + v, + ...extra + }: IndexDescriptionInfo & { + name: string; + }, indexStats?: IndexStats, indexSize?: number, maxSize?: number, @@ -150,7 +178,7 @@ export function createIndexDefinition( ...indexStats, type: getIndexType(index), cardinality: getIndexCardinality(index), - properties: getIndexProperties(index), + properties: getIndexProperties(index, collectionShardKey), size: indexSize, relativeSize: (indexSize / maxSize) * 100, buildProgress: buildProgress ?? 0,