SERVER-33456 Ensure `onShardVersionMismatch` does not throw

2 files changed, 14 insertions, 6 deletions

diff --git a/src/mongo/db/s/shard_filtering_metadata_refresh.cpp b/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
index c35eba0e2e2..224dd05ccab 100644
--- a/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
+++ b/src/mongo/db/s/shard_filtering_metadata_refresh.cpp
@@ -58,9 +58,15 @@ Status onShardVersionMismatch(OperationContext* opCtx,
 
     ShardingStatistics::get(opCtx).countStaleConfigErrors.addAndFetch(1);
 
-    // Ensure any ongoing migrations have completed
-    auto& oss = OperationShardingState::get(opCtx);
-    oss.waitForMigrationCriticalSectionSignal(opCtx);
+    // Ensure any ongoing migrations have completed before trying to do the refresh. This wait is
+    // just an optimization so that MongoS does not exhaust its maximum number of StaleConfig retry
+    // attempts while the migration is being committed.
+    try {
+        auto& oss = OperationShardingState::get(opCtx);
+        oss.waitForMigrationCriticalSectionSignal(opCtx);
+    } catch (const DBException& ex) {
+        return ex.toStatus();
+    }
 
     const auto currentShardVersion = [&] {
         AutoGetCollection autoColl(opCtx, nss, MODE_IS);
diff --git a/src/mongo/db/s/shard_filtering_metadata_refresh.h b/src/mongo/db/s/shard_filtering_metadata_refresh.h
index 9d187c49449..2fbac64ae5d 100644
--- a/src/mongo/db/s/shard_filtering_metadata_refresh.h
+++ b/src/mongo/db/s/shard_filtering_metadata_refresh.h
@@ -36,7 +36,7 @@ namespace mongo {
 class OperationContext;
 
 /**
- * Must be invoked whenever code, which is executing on a shard encounters a StaleConfing exception
+ * Must be invoked whenever code, which is executing on a shard encounters a StaleConfig exception
  * and should be passed the 'version received' from the exception. If the shard's current version is
  * behind 'shardVersionReceived', causes the shard's filtering metadata to be refreshed from the
  * config server, otherwise does nothing and immediately returns. If there are other threads
@@ -47,8 +47,10 @@ class OperationContext;
  * NOTE: Does network I/O and acquires collection lock on the specified namespace, so it must not be
  * called with a lock
  *
- * NOTE: None of the callers of this function expect it to throw and it will be invalid to do so,
- * hence the noexcept qualifier.
+ * NOTE: This method is not expected to throw, because it is used in places where StaleConfig
+ * exception was just caught and if it were to throw, it would overwrite any accumulated command
+ * execution state in the response. This is specifically problematic for write commands, which are
+ * expected to return the set of write batch entries that succeeded.
  */
 Status onShardVersionMismatch(OperationContext* opCtx,
                               const NamespaceString& nss,