Skip to content

Commit

Permalink
Merge pull request #656 from cole-miller/lock-directory
Browse files Browse the repository at this point in the history 
Add file locking to prevent two dqlite instances from using the same directory concurrently
  • Loading branch information
@cole-miller
cole-miller authored Jun 25, 2024
2 parents 91267eb + 294464b commit 57c50eb
Show file tree
Hide file tree
Showing 8 changed files with 140 additions and 44 deletions.
9 changes: 6 additions & 3 deletions src/config.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ static unsigned serial = 1;
int config__init(struct config *c,
dqlite_node_id id,
const char *address,
const char *dir)
const char *raft_dir,
const char *database_dir)
{
int rv;
c->id = id;
Expand All @@ -49,8 +50,10 @@ int config__init(struct config *c,
c->logger.emit = loggerDefaultEmit;
c->failure_domain = 0;
c->weight = 0;
strncpy(c->dir, dir, sizeof(c->dir) - 1);
c->dir[sizeof(c->dir) - 1] = '\0';

snprintf(c->raft_dir, sizeof(c->raft_dir), "%s", (raft_dir != NULL) ? raft_dir : "");
snprintf(c->database_dir, sizeof(c->database_dir), "%s", database_dir);

c->disk = false;
c->voters = 3;
c->standbys = 0;
Expand Down
6 changes: 4 additions & 2 deletions src/config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ struct config {
char name[256]; /* VFS/replication registriatio name */
unsigned long long failure_domain; /* User-provided failure domain */
unsigned long long int weight; /* User-provided node weight */
char dir[1024]; /* Data dir for on-disk database */
char raft_dir[1024]; /* Directory used by raft */
char database_dir[1024]; /* Data dir for on-disk database */
bool disk; /* Disk-mode or not */
int voters; /* Target number of voters */
int standbys; /* Target number of standbys */
Expand All @@ -30,7 +31,8 @@ struct config {
int config__init(struct config *c,
dqlite_node_id id,
const char *address,
const char *dir);
const char *raft_dir,
const char *database_dir);

/**
* Release any memory held by the config object.
Expand Down
2 changes: 1 addition & 1 deletion src/db.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ int db__init(struct db *db, struct config *config, const char *filename)
}
if (db->config->disk) {
rv = snprintf(db->path, MAX_PATHNAME + 1, "%s/%s",
db->config->dir, db->filename);
db->config->database_dir, db->filename);
} else {
rv = snprintf(db->path, MAX_PATHNAME + 1, "%s", db->filename);
}
Expand Down
62 changes: 56 additions & 6 deletions src/server.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <errno.h>
#include <sched.h>
#include <stdlib.h>
#include <sys/file.h>
#include <sys/un.h>
#include <time.h>
#include <uv.h>
Expand Down Expand Up @@ -63,6 +64,7 @@ int dqlite__init(struct dqlite_node *d,
ssize_t count;

d->initialized = false;
d->lock_fd = -1;
memset(d->errmsg, 0, sizeof d->errmsg);

rv = snprintf(db_dir_path, sizeof db_dir_path, DATABASE_DIR_FMT, dir);
Expand All @@ -72,7 +74,7 @@ int dqlite__init(struct dqlite_node *d,
goto err;
}

rv = config__init(&d->config, id, address, db_dir_path);
rv = config__init(&d->config, id, address, dir, db_dir_path);
if (rv != 0) {
snprintf(d->errmsg, DQLITE_ERRMSG_BUF_SIZE,
"config__init(rv:%d)", rv);
Expand Down Expand Up @@ -829,6 +831,32 @@ static bool taskReady(struct dqlite_node *d)
return d->running;
}

#define LOCK_FILENAME "dqlite-lock"

static int acquire_dir(const char *dir, int *fd_out)
{
char path[PATH_MAX];
int fd;
int rv;

snprintf(path, sizeof(path), "%s/%s", dir, LOCK_FILENAME);
fd = open(path, O_RDWR|O_CREAT|O_CLOEXEC, S_IRUSR|S_IWUSR);
if (fd < 0) {
return DQLITE_ERROR;
}
rv = flock(fd, LOCK_EX|LOCK_NB);
if (rv != 0) {
return DQLITE_ERROR;
}
*fd_out = fd;
return 0;
}

static void release_dir(int fd)
{
close(fd);
}

static int dqliteDatabaseDirSetup(dqlite_node *t)
{
int rv;
Expand All @@ -837,14 +865,14 @@ static int dqliteDatabaseDirSetup(dqlite_node *t)
return 0;
}

rv = FsEnsureDir(t->config.dir);
rv = FsEnsureDir(t->config.database_dir);
if (rv != 0) {
snprintf(t->errmsg, DQLITE_ERRMSG_BUF_SIZE,
"Error creating database dir: %d", rv);
return rv;
}

rv = FsRemoveDirFiles(t->config.dir);
rv = FsRemoveDirFiles(t->config.database_dir);
if (rv != 0) {
snprintf(t->errmsg, DQLITE_ERRMSG_BUF_SIZE,
"Error removing files in database dir: %d", rv);
Expand All @@ -867,27 +895,39 @@ int dqlite_node_start(dqlite_node *t)
goto err;
}

int lock_fd;
rv = acquire_dir(t->config.raft_dir, &lock_fd);
if (rv != 0) {
snprintf(t->errmsg, DQLITE_ERRMSG_BUF_SIZE,
"couldn't lock the raft directory");
return rv;
}
t->lock_fd = lock_fd;

rv = maybeBootstrap(t, t->config.id, t->config.address);
if (rv != 0) {
tracef("bootstrap failed %d", rv);
goto err;
goto err_after_acquire_dir;
}

rv = pthread_create(&t->thread, 0, &taskStart, t);
if (rv != 0) {
tracef("pthread create failed %d", rv);
rv = DQLITE_ERROR;
goto err;
goto err_after_acquire_dir;
}

if (!taskReady(t)) {
tracef("!taskReady");
rv = DQLITE_ERROR;
goto err;
goto err_after_acquire_dir;
}

return 0;


err_after_acquire_dir:
release_dir(t->lock_fd);
err:
return rv;
}
Expand Down Expand Up @@ -916,6 +956,8 @@ int dqlite_node_stop(dqlite_node *d)
rv = pthread_join(d->thread, &result);
assert(rv == 0);

release_dir(d->lock_fd);

return (int)((uintptr_t)result);
}

Expand Down Expand Up @@ -1004,12 +1046,20 @@ int dqlite_node_recover_ext(dqlite_node *n,
};
}

int lock_fd;
rv = acquire_dir(n->config.raft_dir, &lock_fd);
if (rv != 0) {
goto out;
}

rv = raft_recover(&n->raft, &configuration);
if (rv != 0) {
rv = DQLITE_ERROR;
goto out;
}

release_dir(lock_fd);

out:
raft_configuration_close(&configuration);
return rv;
Expand Down
1 change: 1 addition & 0 deletions src/server.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
struct dqlite_node {
bool initialized; /* dqlite__init succeeded */

int lock_fd; /* Locked file in the data directory */
pthread_t thread; /* Main run loop thread. */
struct config config; /* Config values */
struct sqlite3_vfs vfs; /* In-memory VFS */
Expand Down
40 changes: 40 additions & 0 deletions test/integration/test_node.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,46 @@ TEST(node, blockSizeRunning, setUp, tearDown, 0, NULL)
return MUNIT_OK;
}

/* Our file locking prevents starting a second dqlite instance that
* uses the same directory as a running instance. */
TEST(node, locked, setUp, tearDown, 0, NULL)
{
struct fixture *f = data;
int rv;

dqlite_node *node2;
rv = dqlite_node_create(2, "2", f->dir, &node2);
munit_assert_int(rv, ==, 0);

rv = dqlite_node_set_bind_address(node2, "@456");
munit_assert_int(rv, ==, 0);

rv = dqlite_node_start(f->node);
munit_assert_int(rv, ==, 0);

char buf[PATH_MAX];
snprintf(buf, sizeof(buf), "%s/dqlite-lock", f->dir);
rv = access(buf, F_OK);
munit_assert_int(rv, ==, 0);

rv = dqlite_node_start(node2);
munit_assert_int(rv, ==, DQLITE_ERROR);
munit_assert_string_equal(dqlite_node_errmsg(node2),
"couldn't lock the raft directory");

rv = dqlite_node_stop(f->node);
munit_assert_int(rv, ==, 0);

rv = dqlite_node_start(node2);
munit_assert_int(rv, ==, 0);

rv = dqlite_node_stop(node2);
munit_assert_int(rv, ==, 0);
dqlite_node_destroy(node2);

return MUNIT_OK;
}

/******************************************************************************
*
* dqlite_node_recover
Expand Down
52 changes: 26 additions & 26 deletions test/lib/cluster.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,32 +68,32 @@ struct server
munit_assert_int(_rv, ==, 0); \
}

#define SETUP_SERVER(I, VERSION) \
{ \
struct server *_s = &f->servers[I]; \
struct raft_fsm *_fsm = &f->fsms[I]; \
char address[16]; \
int _rc; \
\
test_logger_setup(params, &_s->logger); \
\
sprintf(address, "%d", I + 1); \
\
char *dir = test_dir_setup(); \
_s->dir = dir; \
\
_rc = config__init(&_s->config, I + 1, address, dir); \
munit_assert_int(_rc, ==, 0); \
\
registry__init(&_s->registry, &_s->config); \
\
_rc = VfsInit(&_s->vfs, _s->config.name); \
munit_assert_int(_rc, ==, 0); \
_rc = sqlite3_vfs_register(&_s->vfs, 0); \
munit_assert_int(_rc, ==, 0); \
\
_rc = fsm__init(_fsm, &_s->config, &_s->registry); \
munit_assert_int(_rc, ==, 0); \
#define SETUP_SERVER(I, VERSION) \
{ \
struct server *_s = &f->servers[I]; \
struct raft_fsm *_fsm = &f->fsms[I]; \
char address[16]; \
int _rc; \
\
test_logger_setup(params, &_s->logger); \
\
sprintf(address, "%d", I + 1); \
\
char *dir = test_dir_setup(); \
_s->dir = dir; \
\
_rc = config__init(&_s->config, I + 1, address, NULL, dir); \
munit_assert_int(_rc, ==, 0); \
\
registry__init(&_s->registry, &_s->config); \
\
_rc = VfsInit(&_s->vfs, _s->config.name); \
munit_assert_int(_rc, ==, 0); \
_rc = sqlite3_vfs_register(&_s->vfs, 0); \
munit_assert_int(_rc, ==, 0); \
\
_rc = fsm__init(_fsm, &_s->config, &_s->registry); \
munit_assert_int(_rc, ==, 0); \
}

#define TEAR_DOWN_CLUSTER \
Expand Down
12 changes: 6 additions & 6 deletions test/lib/config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@

#define FIXTURE_CONFIG struct config config;

#define SETUP_CONFIG \
{ \
int rc; \
rc = config__init(&f->config, 1, "1", "dir"); \
munit_assert_int(rc, ==, 0); \
test_logger_setup(params, &f->config.logger); \
#define SETUP_CONFIG \
{ \
int rc; \
rc = config__init(&f->config, 1, "1", NULL, "dir"); \
munit_assert_int(rc, ==, 0); \
test_logger_setup(params, &f->config.logger); \
}

#define TEAR_DOWN_CONFIG \
Expand Down

0 comments on commit 57c50eb

Please sign in to comment.