From 1c0808035cc0a1f544e89c48e3f04cdd73cb2311 Mon Sep 17 00:00:00 2001 From: John Gibbons Date: Fri, 6 Oct 2017 10:56:13 +0100 Subject: [PATCH 1/2] CSHARP-2043: Added SuppressEnsureIndexes option to GridFSBucketOptions. By default is false and has no impact on any part of the system. If set to true, then suppresses the call to EnsureIndexes which occurs during every GridFS upload call. Benefit of this suppression is that: 1) EnsureIndexes [effectively] does a count against the fs.files collection, which will a) add latency in a cluster with global shards, b) requires find permission which may not be deisrable in a write-only repository and most importantly c) cause entire cluster to become unavailable if any single shard is unavailable, even if data being saved is not housed in the unavailable shard. 2) Less importantly, EnsureIndexes includes a list index command which implies a higher privilege than is really needed. If developer is a) confident that GridFS subsystem is properly set up and b) wants to robustify sharded GridFS against one shard being down c) wants keep client permissions to a minimum --- src/MongoDB.Driver.GridFS/GridFSBucket.cs | 38 ++++++++++-------- .../GridFSBucketOptions.cs | 29 ++++++++++++++ .../GridFSBucketOptionsTests.cs | 39 ++++++++++++++++++- 3 files changed, 88 insertions(+), 18 deletions(-) diff --git a/src/MongoDB.Driver.GridFS/GridFSBucket.cs b/src/MongoDB.Driver.GridFS/GridFSBucket.cs index 9313635462d..ab82a893d5a 100644 --- a/src/MongoDB.Driver.GridFS/GridFSBucket.cs +++ b/src/MongoDB.Driver.GridFS/GridFSBucket.cs @@ -814,16 +814,19 @@ private void EnsureIndexes(IReadWriteBindingHandle binding, CancellationToken ca { if (!_ensureIndexesDone) { - var isFilesCollectionEmpty = IsFilesCollectionEmpty(binding, cancellationToken); - if (isFilesCollectionEmpty) + if (!_options.SuppressEnsureIndexes) { - if (!FilesCollectionIndexesExist(binding, cancellationToken)) + var isFilesCollectionEmpty = IsFilesCollectionEmpty(binding, cancellationToken); + if (isFilesCollectionEmpty) { - CreateFilesCollectionIndexes(binding, cancellationToken); - } - if (!ChunksCollectionIndexesExist(binding, cancellationToken)) - { - CreateChunksCollectionIndexes(binding, cancellationToken); + if (!FilesCollectionIndexesExist(binding, cancellationToken)) + { + CreateFilesCollectionIndexes(binding, cancellationToken); + } + if (!ChunksCollectionIndexesExist(binding, cancellationToken)) + { + CreateChunksCollectionIndexes(binding, cancellationToken); + } } } @@ -843,16 +846,19 @@ private async Task EnsureIndexesAsync(IReadWriteBindingHandle binding, Cancellat { if (!_ensureIndexesDone) { - var isFilesCollectionEmpty = await IsFilesCollectionEmptyAsync(binding, cancellationToken).ConfigureAwait(false); - if (isFilesCollectionEmpty) + if (!_options.SuppressEnsureIndexes) { - if (!(await FilesCollectionIndexesExistAsync(binding, cancellationToken).ConfigureAwait(false))) - { - await CreateFilesCollectionIndexesAsync(binding, cancellationToken).ConfigureAwait(false); - } - if (!(await ChunksCollectionIndexesExistAsync(binding, cancellationToken).ConfigureAwait(false))) + var isFilesCollectionEmpty = await IsFilesCollectionEmptyAsync(binding, cancellationToken).ConfigureAwait(false); + if (isFilesCollectionEmpty) { - await CreateChunksCollectionIndexesAsync(binding, cancellationToken).ConfigureAwait(false); + if (!(await FilesCollectionIndexesExistAsync(binding, cancellationToken).ConfigureAwait(false))) + { + await CreateFilesCollectionIndexesAsync(binding, cancellationToken).ConfigureAwait(false); + } + if (!(await ChunksCollectionIndexesExistAsync(binding, cancellationToken).ConfigureAwait(false))) + { + await CreateChunksCollectionIndexesAsync(binding, cancellationToken).ConfigureAwait(false); + } } } diff --git a/src/MongoDB.Driver.GridFS/GridFSBucketOptions.cs b/src/MongoDB.Driver.GridFS/GridFSBucketOptions.cs index 145d3f4733b..bc66c7c7362 100644 --- a/src/MongoDB.Driver.GridFS/GridFSBucketOptions.cs +++ b/src/MongoDB.Driver.GridFS/GridFSBucketOptions.cs @@ -29,6 +29,7 @@ public class GridFSBucketOptions private ReadConcern _readConcern; private ReadPreference _readPreference; private WriteConcern _writeConcern; + private bool _suppressEnsureIndexes; // constructors /// @@ -51,6 +52,7 @@ public GridFSBucketOptions(GridFSBucketOptions other) _readConcern = other.ReadConcern; _readPreference = other.ReadPreference; _writeConcern = other.WriteConcern; + _suppressEnsureIndexes = other.SuppressEnsureIndexes; } /// @@ -65,6 +67,7 @@ public GridFSBucketOptions(ImmutableGridFSBucketOptions other) _readConcern = other.ReadConcern; _readPreference = other.ReadPreference; _writeConcern = other.WriteConcern; + _suppressEnsureIndexes = other.SuppressEnsureIndexes; } // properties @@ -135,6 +138,18 @@ public WriteConcern WriteConcern get { return _writeConcern; } set { _writeConcern = value; } } + + /// + /// Gets or sets the suppress ensure indexes setting + /// + /// + /// The suppress ensure indexes setting + /// + public bool SuppressEnsureIndexes + { + get { return _suppressEnsureIndexes; } + set { _suppressEnsureIndexes = value; } + } } /// @@ -165,6 +180,7 @@ public static ImmutableGridFSBucketOptions Defaults private readonly ReadConcern _readConcern; private readonly ReadPreference _readPreference; private readonly WriteConcern _writeConcern; + private readonly bool _suppressEnsureIndexes; // constructors /// @@ -174,6 +190,7 @@ public ImmutableGridFSBucketOptions() { _bucketName = "fs"; _chunkSizeBytes = 255 * 1024; + _suppressEnsureIndexes = false; } /// @@ -188,6 +205,7 @@ public ImmutableGridFSBucketOptions(GridFSBucketOptions other) _readConcern = other.ReadConcern; _readPreference = other.ReadPreference; _writeConcern = other.WriteConcern; + _suppressEnsureIndexes = other.SuppressEnsureIndexes; } // properties @@ -256,5 +274,16 @@ public WriteConcern WriteConcern { get { return _writeConcern; } } + + /// + /// Gets the suppress ensure indexes setting + /// + /// + /// The suppress ensure indexes setting + /// + public bool SuppressEnsureIndexes + { + get { return _suppressEnsureIndexes; } + } } } diff --git a/tests/MongoDB.Driver.GridFS.Tests/GridFSBucketOptionsTests.cs b/tests/MongoDB.Driver.GridFS.Tests/GridFSBucketOptionsTests.cs index e6d9ab3f77b..449ac618b18 100644 --- a/tests/MongoDB.Driver.GridFS.Tests/GridFSBucketOptionsTests.cs +++ b/tests/MongoDB.Driver.GridFS.Tests/GridFSBucketOptionsTests.cs @@ -117,7 +117,7 @@ public void constructor_with_immutable_other_should_initialize_instance() [Fact] public void constructor_with_mutable_other_should_initialize_instance() { - var other = new GridFSBucketOptions { BucketName = "bucket", ChunkSizeBytes = 123, ReadConcern = ReadConcern.Majority, ReadPreference = ReadPreference.Secondary, WriteConcern = WriteConcern.WMajority }; + var other = new GridFSBucketOptions { BucketName = "bucket", ChunkSizeBytes = 123, ReadConcern = ReadConcern.Majority, ReadPreference = ReadPreference.Secondary, WriteConcern = WriteConcern.WMajority, SuppressEnsureIndexes = true }; var result = new GridFSBucketOptions(other); @@ -126,6 +126,7 @@ public void constructor_with_mutable_other_should_initialize_instance() result.ReadConcern.Should().Be(other.ReadConcern); result.ReadPreference.Should().Be(other.ReadPreference); result.WriteConcern.Should().Be(other.WriteConcern); + result.SuppressEnsureIndexes.Should().Be(other.SuppressEnsureIndexes); } [Fact] @@ -137,6 +138,7 @@ public void constructor_with_no_arguments_should_initialize_instance_with_defaul result.ChunkSizeBytes.Should().Be(255 * 1024); result.ReadPreference.Should().BeNull(); result.WriteConcern.Should().BeNull(); + result.SuppressEnsureIndexes.Should().BeFalse(); } [Fact] @@ -198,6 +200,26 @@ public void WriteConcern_set_should_have_expected_result() subject.WriteConcern.Should().Be(WriteConcern.WMajority); } + + [Fact] + public void SuppressEnsureIndexes_get_should_return_expected_result() + { + var subject = new GridFSBucketOptions { SuppressEnsureIndexes = true }; + + var result = subject.SuppressEnsureIndexes; + + result.Should().BeTrue(); + } + + [Fact] + public void SuppressEnsureIndexes_set_should_have_expected_result() + { + var subject = new GridFSBucketOptions(); + + subject.SuppressEnsureIndexes = true; + + subject.SuppressEnsureIndexes.Should().BeTrue(); + } } public class ImmutableGridFSBucketOptionsTests @@ -225,7 +247,7 @@ public void ChunkSizeBytes_get_should_return_expected_result() [Fact] public void constructor_with_arguments_should_initialize_instance() { - var mutable = new GridFSBucketOptions { BucketName = "bucket", ChunkSizeBytes = 123, ReadConcern = ReadConcern.Majority, ReadPreference = ReadPreference.Secondary, WriteConcern = WriteConcern.WMajority }; + var mutable = new GridFSBucketOptions { BucketName = "bucket", ChunkSizeBytes = 123, ReadConcern = ReadConcern.Majority, ReadPreference = ReadPreference.Secondary, WriteConcern = WriteConcern.WMajority, SuppressEnsureIndexes = true }; var result = new ImmutableGridFSBucketOptions(mutable); @@ -234,6 +256,7 @@ public void constructor_with_arguments_should_initialize_instance() result.ReadConcern.Should().Be(ReadConcern.Majority); result.ReadPreference.Should().Be(ReadPreference.Secondary); result.WriteConcern.Should().Be(WriteConcern.WMajority); + result.SuppressEnsureIndexes.Should().BeTrue(); } [Fact] @@ -246,6 +269,7 @@ public void constructor_with_no_arguments_should_initialize_instance_with_defaul result.ReadConcern.Should().BeNull(); result.ReadPreference.Should().BeNull(); result.WriteConcern.Should().BeNull(); + result.SuppressEnsureIndexes.Should().BeFalse(); } [Fact] @@ -267,6 +291,7 @@ public void Defaults_get_should_return_expected_result() result.ReadConcern.Should().BeNull(); result.ReadPreference.Should().BeNull(); result.WriteConcern.Should().BeNull(); + result.SuppressEnsureIndexes.Should().BeFalse(); } [Fact] @@ -298,5 +323,15 @@ public void WriteConcern_get_should_return_expected_result() result.Should().Be(WriteConcern.WMajority); } + + [Fact] + public void SuppressEnsureIndexes_get_should_return_expected_result() + { + var subject = new ImmutableGridFSBucketOptions(new GridFSBucketOptions { SuppressEnsureIndexes = true }); + + var result = subject.SuppressEnsureIndexes; + + result.Should().BeTrue(); + } } } From fc576557daea04edb040ef8835ea873dbb63e2eb Mon Sep 17 00:00:00 2001 From: John Gibbons Date: Wed, 11 Oct 2017 14:43:46 +0100 Subject: [PATCH 2/2] CSHARP-2043: used better name for option: was SuppressEnsureIndexes, now AssumeIndexesExist --- src/MongoDB.Driver.GridFS/GridFSBucket.cs | 4 +-- .../GridFSBucketOptions.cs | 30 ++++++++--------- .../GridFSBucketOptionsTests.cs | 32 +++++++++---------- 3 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/MongoDB.Driver.GridFS/GridFSBucket.cs b/src/MongoDB.Driver.GridFS/GridFSBucket.cs index ab82a893d5a..3b924a9eab5 100644 --- a/src/MongoDB.Driver.GridFS/GridFSBucket.cs +++ b/src/MongoDB.Driver.GridFS/GridFSBucket.cs @@ -814,7 +814,7 @@ private void EnsureIndexes(IReadWriteBindingHandle binding, CancellationToken ca { if (!_ensureIndexesDone) { - if (!_options.SuppressEnsureIndexes) + if (!_options.AssumeIndexesExist) { var isFilesCollectionEmpty = IsFilesCollectionEmpty(binding, cancellationToken); if (isFilesCollectionEmpty) @@ -846,7 +846,7 @@ private async Task EnsureIndexesAsync(IReadWriteBindingHandle binding, Cancellat { if (!_ensureIndexesDone) { - if (!_options.SuppressEnsureIndexes) + if (!_options.AssumeIndexesExist) { var isFilesCollectionEmpty = await IsFilesCollectionEmptyAsync(binding, cancellationToken).ConfigureAwait(false); if (isFilesCollectionEmpty) diff --git a/src/MongoDB.Driver.GridFS/GridFSBucketOptions.cs b/src/MongoDB.Driver.GridFS/GridFSBucketOptions.cs index bc66c7c7362..702cdefa2cc 100644 --- a/src/MongoDB.Driver.GridFS/GridFSBucketOptions.cs +++ b/src/MongoDB.Driver.GridFS/GridFSBucketOptions.cs @@ -29,7 +29,7 @@ public class GridFSBucketOptions private ReadConcern _readConcern; private ReadPreference _readPreference; private WriteConcern _writeConcern; - private bool _suppressEnsureIndexes; + private bool _assumeIndexesExist; // constructors /// @@ -52,7 +52,7 @@ public GridFSBucketOptions(GridFSBucketOptions other) _readConcern = other.ReadConcern; _readPreference = other.ReadPreference; _writeConcern = other.WriteConcern; - _suppressEnsureIndexes = other.SuppressEnsureIndexes; + _assumeIndexesExist = other.AssumeIndexesExist; } /// @@ -67,7 +67,7 @@ public GridFSBucketOptions(ImmutableGridFSBucketOptions other) _readConcern = other.ReadConcern; _readPreference = other.ReadPreference; _writeConcern = other.WriteConcern; - _suppressEnsureIndexes = other.SuppressEnsureIndexes; + _assumeIndexesExist = other.AssumeIndexesExist; } // properties @@ -140,15 +140,15 @@ public WriteConcern WriteConcern } /// - /// Gets or sets the suppress ensure indexes setting + /// Gets or sets the assume indexes exist setting /// /// - /// The suppress ensure indexes setting + /// The assume indexes exist setting /// - public bool SuppressEnsureIndexes + public bool AssumeIndexesExist { - get { return _suppressEnsureIndexes; } - set { _suppressEnsureIndexes = value; } + get { return _assumeIndexesExist; } + set { _assumeIndexesExist = value; } } } @@ -180,7 +180,7 @@ public static ImmutableGridFSBucketOptions Defaults private readonly ReadConcern _readConcern; private readonly ReadPreference _readPreference; private readonly WriteConcern _writeConcern; - private readonly bool _suppressEnsureIndexes; + private readonly bool _assumeIndexesExist; // constructors /// @@ -190,7 +190,7 @@ public ImmutableGridFSBucketOptions() { _bucketName = "fs"; _chunkSizeBytes = 255 * 1024; - _suppressEnsureIndexes = false; + _assumeIndexesExist = false; } /// @@ -205,7 +205,7 @@ public ImmutableGridFSBucketOptions(GridFSBucketOptions other) _readConcern = other.ReadConcern; _readPreference = other.ReadPreference; _writeConcern = other.WriteConcern; - _suppressEnsureIndexes = other.SuppressEnsureIndexes; + _assumeIndexesExist = other.AssumeIndexesExist; } // properties @@ -276,14 +276,14 @@ public WriteConcern WriteConcern } /// - /// Gets the suppress ensure indexes setting + /// Gets the assume ensure indexes setting /// /// - /// The suppress ensure indexes setting + /// The assume ensure indexes setting /// - public bool SuppressEnsureIndexes + public bool AssumeIndexesExist { - get { return _suppressEnsureIndexes; } + get { return _assumeIndexesExist; } } } } diff --git a/tests/MongoDB.Driver.GridFS.Tests/GridFSBucketOptionsTests.cs b/tests/MongoDB.Driver.GridFS.Tests/GridFSBucketOptionsTests.cs index 449ac618b18..8d182a9ba42 100644 --- a/tests/MongoDB.Driver.GridFS.Tests/GridFSBucketOptionsTests.cs +++ b/tests/MongoDB.Driver.GridFS.Tests/GridFSBucketOptionsTests.cs @@ -117,7 +117,7 @@ public void constructor_with_immutable_other_should_initialize_instance() [Fact] public void constructor_with_mutable_other_should_initialize_instance() { - var other = new GridFSBucketOptions { BucketName = "bucket", ChunkSizeBytes = 123, ReadConcern = ReadConcern.Majority, ReadPreference = ReadPreference.Secondary, WriteConcern = WriteConcern.WMajority, SuppressEnsureIndexes = true }; + var other = new GridFSBucketOptions { BucketName = "bucket", ChunkSizeBytes = 123, ReadConcern = ReadConcern.Majority, ReadPreference = ReadPreference.Secondary, WriteConcern = WriteConcern.WMajority, AssumeIndexesExist = true }; var result = new GridFSBucketOptions(other); @@ -126,7 +126,7 @@ public void constructor_with_mutable_other_should_initialize_instance() result.ReadConcern.Should().Be(other.ReadConcern); result.ReadPreference.Should().Be(other.ReadPreference); result.WriteConcern.Should().Be(other.WriteConcern); - result.SuppressEnsureIndexes.Should().Be(other.SuppressEnsureIndexes); + result.AssumeIndexesExist.Should().Be(other.AssumeIndexesExist); } [Fact] @@ -138,7 +138,7 @@ public void constructor_with_no_arguments_should_initialize_instance_with_defaul result.ChunkSizeBytes.Should().Be(255 * 1024); result.ReadPreference.Should().BeNull(); result.WriteConcern.Should().BeNull(); - result.SuppressEnsureIndexes.Should().BeFalse(); + result.AssumeIndexesExist.Should().BeFalse(); } [Fact] @@ -202,23 +202,23 @@ public void WriteConcern_set_should_have_expected_result() } [Fact] - public void SuppressEnsureIndexes_get_should_return_expected_result() + public void AssumeIndexesExist_get_should_return_expected_result() { - var subject = new GridFSBucketOptions { SuppressEnsureIndexes = true }; + var subject = new GridFSBucketOptions { AssumeIndexesExist = true }; - var result = subject.SuppressEnsureIndexes; + var result = subject.AssumeIndexesExist; result.Should().BeTrue(); } [Fact] - public void SuppressEnsureIndexes_set_should_have_expected_result() + public void AssumeIndexesExist_set_should_have_expected_result() { var subject = new GridFSBucketOptions(); - subject.SuppressEnsureIndexes = true; + subject.AssumeIndexesExist = true; - subject.SuppressEnsureIndexes.Should().BeTrue(); + subject.AssumeIndexesExist.Should().BeTrue(); } } @@ -247,7 +247,7 @@ public void ChunkSizeBytes_get_should_return_expected_result() [Fact] public void constructor_with_arguments_should_initialize_instance() { - var mutable = new GridFSBucketOptions { BucketName = "bucket", ChunkSizeBytes = 123, ReadConcern = ReadConcern.Majority, ReadPreference = ReadPreference.Secondary, WriteConcern = WriteConcern.WMajority, SuppressEnsureIndexes = true }; + var mutable = new GridFSBucketOptions { BucketName = "bucket", ChunkSizeBytes = 123, ReadConcern = ReadConcern.Majority, ReadPreference = ReadPreference.Secondary, WriteConcern = WriteConcern.WMajority, AssumeIndexesExist = true }; var result = new ImmutableGridFSBucketOptions(mutable); @@ -256,7 +256,7 @@ public void constructor_with_arguments_should_initialize_instance() result.ReadConcern.Should().Be(ReadConcern.Majority); result.ReadPreference.Should().Be(ReadPreference.Secondary); result.WriteConcern.Should().Be(WriteConcern.WMajority); - result.SuppressEnsureIndexes.Should().BeTrue(); + result.AssumeIndexesExist.Should().BeTrue(); } [Fact] @@ -269,7 +269,7 @@ public void constructor_with_no_arguments_should_initialize_instance_with_defaul result.ReadConcern.Should().BeNull(); result.ReadPreference.Should().BeNull(); result.WriteConcern.Should().BeNull(); - result.SuppressEnsureIndexes.Should().BeFalse(); + result.AssumeIndexesExist.Should().BeFalse(); } [Fact] @@ -291,7 +291,7 @@ public void Defaults_get_should_return_expected_result() result.ReadConcern.Should().BeNull(); result.ReadPreference.Should().BeNull(); result.WriteConcern.Should().BeNull(); - result.SuppressEnsureIndexes.Should().BeFalse(); + result.AssumeIndexesExist.Should().BeFalse(); } [Fact] @@ -325,11 +325,11 @@ public void WriteConcern_get_should_return_expected_result() } [Fact] - public void SuppressEnsureIndexes_get_should_return_expected_result() + public void AssumeIndexesExist_get_should_return_expected_result() { - var subject = new ImmutableGridFSBucketOptions(new GridFSBucketOptions { SuppressEnsureIndexes = true }); + var subject = new ImmutableGridFSBucketOptions(new GridFSBucketOptions { AssumeIndexesExist = true }); - var result = subject.SuppressEnsureIndexes; + var result = subject.AssumeIndexesExist; result.Should().BeTrue(); }