Skip to content

Commit

Permalink
SERVER-7775 refactor fsyncUnlock to execute through standard command …
Browse files Browse the repository at this point in the history
…path
  • Loading branch information
amidvidy committed Mar 26, 2015
1 parent 78005d5 commit 8a80559
Show file tree
Hide file tree
Showing 14 changed files with 177 additions and 64 deletions.
3 changes: 2 additions & 1 deletion jstests/auth/auth3.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
var conn = MongoRunner.runMongod({auth : "", port : 31001});

var admin = conn.getDB("admin");
var errorCodeUnauthorized = 13;

admin.createUser({user:"foo",pwd: "bar", roles: jsTest.adminUserRoles});

Expand All @@ -16,7 +17,7 @@ assert.eq(x.err, "unauthorized", tojson(x));

x = admin.fsyncUnlock();
assert(x.errmsg != "not locked", tojson(x));
assert.eq(x.err, "unauthorized", tojson(x));
assert.eq(x.code, errorCodeUnauthorized, tojson(x));

conn.getDB("admin").auth("foo","bar");

Expand Down
3 changes: 2 additions & 1 deletion jstests/auth/basic_role_auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,9 @@ var testOps = function(db, allowedActions) {
if (!isMongos){
checkErr(allowedActions.hasOwnProperty('fsync_unlock'), function() {
var res = db.fsyncUnlock();
var errorCodeUnauthorized = 13;

if (res.err == 'unauthorized') {
if (res.code == errorCodeUnauthorized) {
throw Error("unauthorized unauthorized fsyncUnlock");
}
});
Expand Down
17 changes: 17 additions & 0 deletions jstests/auth/lib/commands_lib.js
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,23 @@ var authCommandsLib = {
{ runOnDb: secondDbName, roles: {} }
]
},
{
testname: "fsyncUnlock",
command: {fsyncUnlock: 1},
skipSharded: true, // TODO: remove when fsyncUnlock is implemented in mongos
testcases: [
{
runOnDb: adminDbName,
roles: roles_hostManager,
privileges: [
{ resource: {cluster: true}, actions: ["unlock"] }
],
expectFail: true
},
{ runOnDb: firstDbName, roles: {} },
{ runOnDb: secondDbName, roles: {} }
]
},
{
testname: "geoNear",
command: {geoNear: "x", near: [50, 50], num: 1},
Expand Down
11 changes: 11 additions & 0 deletions jstests/core/fsync.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,14 @@ assert.eq(2, fsyncLockDB.coll.count({}));

// Ensure eval is not allowed to invoke fsyncLock
assert(!db.eval('db.fsyncLock()').ok, "eval('db.fsyncLock()') should fail.");

// Check that the fsyncUnlock pseudo-command (a lookup on cmd.$sys.unlock)
// still has the same effect as a legitimate 'fsyncUnlock' command
// TODO: remove this in in the release following MongoDB 3.2 when pseudo-commands
// are removed
var fsyncCommandRes = db.fsyncLock();
assert(fsyncLockRes.ok, "fsyncLock command failed against admin DB");
assert(db.currentOp().fsyncLock, "Value in db.currentOp incorrect for fsyncLocked server");
var fsyncPseudoCommandRes = db.getSiblingDB("admin").$cmd.sys.unlock.findOne();
assert(fsyncPseudoCommandRes.ok, "fsyncUnlock pseudo-command failed");
assert(db.currentOp().fsyncLock == null, "fsyncUnlock is not null in db.currentOp");
2 changes: 1 addition & 1 deletion jstests/noPassthroughWithMongod/fsync2.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ function doTest() {
// Uncomment once SERVER-4243 is fixed
//assert.eq(1, m.getDB(db.getName()).fsync2.count());

assert( m.getDB("admin").$cmd.sys.unlock.findOne().ok );
assert( m.getDB("admin").fsyncUnlock().ok );

assert.eq( 2, db.fsync2.count() );

Expand Down
4 changes: 2 additions & 2 deletions jstests/repl/snapshot1.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ for( i = 0; i < 1000; ++i )

m.getDB( "admin" ).runCommand( {fsync:1,lock:1} );
copyDbpath( rt1.getPath( true ), rt1.getPath( false ) );
m.getDB( "admin" ).$cmd.sys.unlock.findOne();
m.getDB( "admin" ).fsyncUnlock();

s1 = rt1.start( false, null, true );
assert.eq( 1000, s1.getDB( baseName )[ baseName ].count() );
Expand All @@ -23,7 +23,7 @@ assert.soon( function() { return 1001 == s1.getDB( baseName )[ baseName ].count(

s1.getDB( "admin" ).runCommand( {fsync:1,lock:1} );
copyDbpath( rt1.getPath( false ), rt2.getPath( false ) );
s1.getDB( "admin" ).$cmd.sys.unlock.findOne();
s1.getDB( "admin" ).fsyncUnlock();

s2 = rt2.start( false, null, true );
assert.eq( 1001, s2.getDB( baseName )[ baseName ].count() );
Expand Down
2 changes: 1 addition & 1 deletion jstests/replsets/fsync_lock_read_secondaries.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ for (var i=0; i<docNum; i++) {
// this should work just fine.
var slave0count = slaves[0].getDB("foo").bar.count();
assert.eq(slave0count, 100, "Doc count in fsync lock wrong. Expected (=100), found " + slave0count);
assert(slaves[0].getDB("admin").$cmd.sys.unlock.findOne().ok);
assert(slaves[0].getDB("admin").fsyncUnlock().ok);

// The secondary should have equal or more documents than what it had before.
assert.soon(function() {
Expand Down
2 changes: 1 addition & 1 deletion jstests/replsets/maxSyncSourceLagSecs.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,6 @@
return (slaves[1].getDB("foo").bar.count() === 2);
}, "slave should have caught up after syncing to primary.");

assert.commandWorked(slaves[0].getDB("admin").$cmd.sys.unlock.findOne());
assert.commandWorked(slaves[0].getDB("admin").fsyncUnlock());
replTest.stopSet();
}());
2 changes: 1 addition & 1 deletion jstests/replsets/stepdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ assert.eq(r2.ismaster, false);
assert.eq(r2.secondary, true);

print("\nunlock");
printjson(locked.getDB("admin").$cmd.sys.unlock.findOne());
printjson(locked.getDB("admin").fsyncUnlock());

print("\nreset stepped down time");
master.getDB("admin").runCommand({replSetFreeze:0});
Expand Down
2 changes: 1 addition & 1 deletion jstests/replsets/stepdown3.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,5 @@ print("result of gle:");
printjson(result);

// unlock and shut down
printjson(locked.getDB("admin").$cmd.sys.unlock.findOne());
printjson(locked.getDB("admin").fsyncUnlock());
replTest.stopSet();
114 changes: 88 additions & 26 deletions src/mongo/db/commands/fsync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,29 +26,34 @@
* it in the license file.
*/

#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kStorage
#define MONGO_LOG_DEFAULT_COMPONENT ::mongo::logger::LogComponent::kCommand

#include "mongo/db/commands/fsync.h"

#include <iostream>
#include <sstream>
#include <string>
#include <vector>

#include "mongo/base/init.h"
#include "mongo/bson/bsonobj.h"
#include "mongo/bson/bsonobjbuilder.h"
#include "mongo/db/audit.h"
#include "mongo/db/auth/action_set.h"
#include "mongo/db/auth/action_type.h"
#include "mongo/db/auth/authorization_manager.h"
#include "mongo/db/auth/authorization_session.h"
#include "mongo/db/auth/privilege.h"
#include "mongo/db/concurrency/d_concurrency.h"
#include "mongo/db/commands.h"
#include "mongo/db/global_environment_experiment.h"
#include "mongo/db/storage/mmap_v1/dur.h"
#include "mongo/db/storage/storage_engine.h"
#include "mongo/db/client.h"
#include "mongo/db/jsobj.h"
#include "mongo/db/operation_context_impl.h"
#include "mongo/util/background.h"
#include "mongo/util/log.h"


namespace mongo {

using std::endl;
Expand Down Expand Up @@ -154,6 +159,61 @@ namespace mongo {
}
} fsyncCmd;

namespace {
bool unlockFsync();
} // namespace

class FSyncUnlockCommand : public Command {
public:

FSyncUnlockCommand() : Command("fsyncUnlock") {}

bool isWriteCommandForConfigServer() const override { return false; }

bool slaveOk() const override { return true; }

bool adminOnly() const override { return true; }

Status checkAuthForCommand(ClientBasic* client,
const std::string& dbname,
const BSONObj& cmdObj) override {

bool isAuthorized = client->getAuthorizationSession()->isAuthorizedForActionsOnResource(
ResourcePattern::forClusterResource(),
ActionType::unlock);

if (isAuthorized) {
audit::logFsyncUnlockAuthzCheck(client, ErrorCodes::OK);
return Status::OK();
}
else {
audit::logFsyncUnlockAuthzCheck(client, ErrorCodes::Unauthorized);
return Status(ErrorCodes::Unauthorized, "Unauthorized");
}
}

bool run(OperationContext* txn,
const std::string& db,
BSONObj& cmdObj,
int options,
std::string& errmsg,
BSONObjBuilder& result,
bool fromRepl) override {

log() << "command: unlock requested";

if (unlockFsync()) {
result.append("info", "unlock completed");
return true;
}
else {
errmsg = "not locked";
return false;
}
}

} unlockFsyncCmd;

SimpleMutex filesLockedFsync("filesLockedFsync");

void FSyncLockThread::doRealWork() {
Expand All @@ -164,12 +224,12 @@ namespace mongo {
Lock::GlobalWrite global(txn.lockState()); // No WriteUnitOfWork needed

SimpleMutex::scoped_lock lk(fsyncCmd.m);

invariant(!fsyncCmd.locked); // impossible to get here if locked is true
try {
try {
getDur().syncDataAndTruncateJournal(&txn);
}
catch( std::exception& e ) {
}
catch( std::exception& e ) {
error() << "error doing syncDataAndTruncateJournal: " << e.what() << endl;
fsyncCmd.err = e.what();
fsyncCmd._threadSync.notify_one();
Expand All @@ -183,7 +243,7 @@ namespace mongo {
StorageEngine* storageEngine = getGlobalEnvironment()->getGlobalStorageEngine();
storageEngine->flushAllFiles(true);
}
catch( std::exception& e ) {
catch( std::exception& e ) {
error() << "error doing flushAll: " << e.what() << endl;
fsyncCmd.err = e.what();
fsyncCmd._threadSync.notify_one();
Expand All @@ -193,37 +253,39 @@ namespace mongo {

invariant(!fsyncCmd.locked);
fsyncCmd.locked = true;

fsyncCmd._threadSync.notify_one();

while ( ! fsyncCmd.pendingUnlock ) {
fsyncCmd._unlockSync.wait(fsyncCmd.m);
}
fsyncCmd.pendingUnlock = false;

fsyncCmd.locked = false;
fsyncCmd.err = "unlocked";

fsyncCmd._unlockSync.notify_one();
}

bool lockedForWriting() {
return fsyncCmd.locked;
bool lockedForWriting() {
return fsyncCmd.locked;
}

// @return true if unlocked
bool _unlockFsync() {
SimpleMutex::scoped_lock lk( fsyncCmd.m );
if( !fsyncCmd.locked ) {
return false;
}
fsyncCmd.pendingUnlock = true;
fsyncCmd._unlockSync.notify_one();
fsyncCmd._threadSync.notify_one();

while ( fsyncCmd.locked ) {
fsyncCmd._unlockSync.wait( fsyncCmd.m );
namespace {
// @return true if unlocked
bool unlockFsync() {
SimpleMutex::scoped_lock lk( fsyncCmd.m );
if( !fsyncCmd.locked ) {
return false;
}
fsyncCmd.pendingUnlock = true;
fsyncCmd._unlockSync.notify_one();
fsyncCmd._threadSync.notify_one();

while ( fsyncCmd.locked ) {
fsyncCmd._unlockSync.wait( fsyncCmd.m );
}
return true;
}
return true;
}
} // namespace
}
2 changes: 1 addition & 1 deletion src/mongo/db/commands/fsync.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ namespace mongo {
// Use this for blocking during an fsync-and-lock
extern SimpleMutex filesLockedFsync;
bool lockedForWriting();
}
} // namespace mongo
Loading

0 comments on commit 8a80559

Please sign in to comment.