Permalink
Browse files

SERVER-5142 double-check in lock for new version before recreating Sh…

…ardChunkManager
  • Loading branch information...
Greg Studer authored and erh committed Mar 2, 2012
1 parent 446f597 commit edc6a4892119009b514202e4d0c97d5c3bf54e1d
Showing with 25 additions and 2 deletions.
  1. +4 −0 s/d_logic.h
  2. +21 −2 s/d_state.cpp
View
@@ -147,6 +147,10 @@ namespace mongo {
// protects state below
mutable mongo::mutex _mutex;
+ // protects accessing the config server
+ // can't use default _mutex b/c it's possible this shard is also the config server,
+ // and we need to access the ShardingState to process new connections
+ mutable mongo::mutex _configServerMutex;
// map from a namespace into the ensemble of chunk ranges that are stored in this mongod
// a ShardChunkManager carries all state we need for a collection at this shard, including its version information
View
@@ -45,7 +45,7 @@ namespace mongo {
// -----ShardingState START ----
ShardingState::ShardingState()
- : _enabled(false) , _mutex( "ShardingState" ) {
+ : _enabled(false) , _mutex( "ShardingState" ), _configServerMutex( "_configServer_ShardingState" ) {
}
void ShardingState::enable( const string& server ) {
@@ -183,7 +183,22 @@ namespace mongo {
bool ShardingState::trySetVersion( const string& ns , ConfigVersion& version /* IN-OUT */ ) {
- // fast path - requested version is at the same version as this chunk manager
+ // Currently this function is called after a getVersion(), which is the first "check", and the assumption here
+ // is that we don't do anything nearly as long as a remote query in a thread between then and now.
+ // Otherwise it may be worth adding an additional check without the _configServerMutex below, since then it
+ // would be likely that the version may have changed in the meantime without waiting for or fetching config results.
+
+ // TODO: Mutex-per-namespace?
+
+ LOG( 2 ) << "trying to set shard version of " << version.toString() << " for '" << ns << "'" << endl;
+
+ scoped_lock clk( _configServerMutex );
+
+ // fast path - double-check if requested version is at the same version as this chunk manager before verifying
+ // against config server
+ //
+ // This path will short-circuit the version set if another thread already managed to update the version in the
+ // meantime. First check is from getVersion().
//
// cases:
// + this shard updated the version for a migrate's commit (FROM side)
@@ -197,6 +212,8 @@ namespace mongo {
if ( it != _chunks.end() && it->second->getVersion() == version )
return true;
}
+
+ LOG( 2 ) << "verifying remote version against " << version.toString() << " for '" << ns << "'" << endl;
// slow path - requested version is different than the current chunk manager's, if one exists, so must check for
// newest version in the config server
@@ -209,8 +226,10 @@ namespace mongo {
// the secondary had no state (managers) at all, so every client request will fall here
// + a stale client request a version that's not current anymore
+ // Can't lock default mutex while creating ShardChunkManager, b/c may have to create a new connection to myself
const string c = (_configServer == _shardHost) ? "" /* local */ : _configServer;
ShardChunkManagerPtr p( new ShardChunkManager( c , ns , _shardName ) );
+
{
scoped_lock lk( _mutex );

0 comments on commit edc6a48

Please sign in to comment.