Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support backing up to an instance of sqlite3.Database #1649

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Support backing up to an instance of sqlite3.Database #1649

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7b74aea
support backing up to an instance of sqlite3.Database
paulfitz Mar 28, 2019
4019379
Resolved several issues and added comments.
Oct 25, 2022
ed7d224
fix: backup-online: database instance was casted to string mistakenly.
Oct 25, 2022
63a1695
fix: added Ref() and Unref() calls to the database instance that was …
Oct 25, 2022
b8c98e3
fix: pass Backup instance to second callback argument
Oct 25, 2022
8d87dcc
ts: added typescript declarations for Backup
Oct 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions lib/sqlite3.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,26 @@ export class Statement extends events.EventEmitter {
each(...params: any[]): this;
}

export class Backup extends events.EventEmitter {
step(pages: number, callback?: (error: Error, backup: Backup) => void): Backup;
finish(callback?: (error: Error, backup: Backup) => void): Backup;

get idle(): boolean;
get completed(): boolean;
get failed(): boolean;
get remaining(): number;
get pageCount(): number;
}

export class Database extends events.EventEmitter {
constructor(filename: string, callback?: (err: Error | null) => void);
constructor(filename: string, mode?: number, callback?: (err: Error | null) => void);

close(callback?: (err: Error | null) => void): void;

backup(destination: string | Database, destName: string, sourceName: string, filenameIsDest = true, callback?: (this: Backup, err: Error | null, backup: Backup) => void): this;
backup(destination: string | Database, callback?: (this: Backup, err: Error | null, backup: Backup) => void): this;

run(sql: string, callback?: (this: RunResult, err: Error | null) => void): this;
run(sql: string, params: any, callback?: (this: RunResult, err: Error | null) => void): this;
run(sql: string, ...params: any[]): this;
Expand Down Expand Up @@ -201,5 +215,6 @@ export interface sqlite3 {
RunResult: RunResult;
Statement: typeof Statement;
Database: typeof Database;
Backup: typeof Backup;
verbose(): this;
}
92 changes: 73 additions & 19 deletions src/backup.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ Backup::Backup(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Backup>(info)
Napi::TypeError::New(env, "Database object expected").ThrowAsJavaScriptException();
return;
}
else if (length <= 1 || !info[1].IsString()) {
Napi::TypeError::New(env, "Filename expected").ThrowAsJavaScriptException();
else if (length <= 1 || !(info[1].IsString() || info[1].IsObject())) {
Napi::TypeError::New(env, "Filename or database object expected").ThrowAsJavaScriptException();
return;
}
else if (length <= 2 || !info[2].IsString()) {
Expand All @@ -155,7 +155,20 @@ Backup::Backup(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Backup>(info)
}

Database* db = Napi::ObjectWrap<Database>::Unwrap(info[0].As<Napi::Object>());
Napi::String filename = info[1].As<Napi::String>();
Database* otherDb = NULL;

Napi::String filename;

if (info[1].IsObject()) {
// A database instance was passed instead of a filename
otherDb = Napi::ObjectWrap<Database>::Unwrap(info[1].As<Napi::Object>());
otherDb->Ref();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if the Ref() call is needed here. I have applied this from @paulfitz.
I assume it is needed because it prevents the otherDb instance to be freed while a backup is in progress, which should usually not happen unless the user closes the db during a backup process.

Please someone double check.


filename = Napi::String::New(env, "<sqlite3 instance>");
} else {
filename = info[1].As<Napi::String>();
}

Napi::String sourceName = info[2].As<Napi::String>();
Napi::String destName = info[3].As<Napi::String>();
Napi::Boolean filenameIsDest = info[4].As<Napi::Boolean>();
Expand All @@ -165,14 +178,28 @@ Backup::Backup(const Napi::CallbackInfo& info) : Napi::ObjectWrap<Backup>(info)
info.This().As<Napi::Object>().DefineProperty(Napi::PropertyDescriptor::Value("destName", destName));
info.This().As<Napi::Object>().DefineProperty(Napi::PropertyDescriptor::Value("filenameIsDest", filenameIsDest));

init(db);
init(db, otherDb);

InitializeBaton* baton = new InitializeBaton(db, info[5].As<Napi::Function>(), this);
baton->otherDb = otherDb;
baton->filename = filename.Utf8Value();
baton->sourceName = sourceName.Utf8Value();
baton->destName = destName.Utf8Value();
baton->filenameIsDest = filenameIsDest.Value();
db->Schedule(Work_BeginInitialize, baton);

if (otherDb) {
otherDb->Schedule(Work_BeforeInitialize, baton, true);
} else {
db->Schedule(Work_BeginInitialize, baton);
}
}

void Backup::Work_BeforeInitialize(Database::Baton* baton) {
InitializeBaton *initBaton = static_cast<InitializeBaton *>(baton);
// at this point, the target database object is locked (it is
// important that its database connection remains unused).
initBaton->otherDb->pending++;
baton->db->Schedule(Work_BeginInitialize, baton);
}

void Backup::Work_BeginInitialize(Database::Baton* baton) {
Expand All @@ -195,22 +222,37 @@ void Backup::Work_Initialize(napi_env e, void* data) {
sqlite3_mutex* mtx = sqlite3_db_mutex(baton->db->_handle);
sqlite3_mutex_enter(mtx);

backup->status = sqlite3_open(baton->filename.c_str(), &backup->_otherDb);
backup->message = "";
if (baton->otherDb) {
// If another database instance was passed,
// link other (locked) db to the backup state
backup->otherDb = baton->otherDb;
backup->_otherDbHandle = baton->otherDb->_handle;

backup->status = SQLITE_OK;
if (!baton->filenameIsDest) {
backup->status = SQLITE_MISUSE;
backup->message = "do not toggle filenameIsDest when backing up between sqlite3.Database instances";
}
} else {
// Do not initialize otherDb and continue with normal
// initialization by using the filename that was provided
backup->otherDb = NULL;
backup->status = sqlite3_open(baton->filename.c_str(), &backup->_otherDbHandle);
}

if (backup->status == SQLITE_OK) {
backup->_handle = sqlite3_backup_init(
baton->filenameIsDest ? backup->_otherDb : backup->db->_handle,
baton->filenameIsDest ? backup->_otherDbHandle : backup->db->_handle,
baton->destName.c_str(),
baton->filenameIsDest ? backup->db->_handle : backup->_otherDb,
baton->filenameIsDest ? backup->db->_handle : backup->_otherDbHandle,
baton->sourceName.c_str());
}
backup->_destDb = baton->filenameIsDest ? backup->_otherDb : backup->db->_handle;
backup->_destDbHandle = baton->filenameIsDest ? backup->_otherDbHandle : backup->db->_handle;

if (backup->status != SQLITE_OK) {
backup->message = std::string(sqlite3_errmsg(backup->_destDb));
sqlite3_close(backup->_otherDb);
backup->_otherDb = NULL;
backup->_destDb = NULL;
if (backup->message == "") backup->message = std::string(sqlite3_errmsg(backup->_destDbHandle));
backup->FinishSqlite();
}

sqlite3_mutex_leave(mtx);
Expand All @@ -231,8 +273,8 @@ void Backup::Work_AfterInitialize(napi_env e, napi_status status, void* data) {
backup->inited = true;
Napi::Function cb = baton->callback.Value();
if (!cb.IsEmpty() && cb.IsFunction()) {
Napi::Value argv[] = { env.Null() };
TRY_CATCH_CALL(backup->Value(), cb, 1, argv);
Napi::Value argv[] = { env.Null(), backup->Value() };
TRY_CATCH_CALL(backup->Value(), cb, 2, argv);
}
}
BACKUP_END();
Expand Down Expand Up @@ -354,18 +396,30 @@ void Backup::FinishAll() {
CleanQueue();
FinishSqlite();
db->Unref();

if (otherDb) {
assert(otherDb->locked);
otherDb->pending--;
otherDb->Process();
otherDb->Unref();
otherDb = NULL;
}
}

void Backup::FinishSqlite() {
if (_handle) {
sqlite3_backup_finish(_handle);
_handle = NULL;
}
if (_otherDb) {
sqlite3_close(_otherDb);
_otherDb = NULL;
if (_otherDbHandle) {
if (!otherDb) {
// Only close the database if it was
// not passed as a descriptor already.
sqlite3_close(_otherDbHandle);
}
_otherDbHandle = NULL;
}
_destDb = NULL;
_destDbHandle = NULL;
}

Napi::Value Backup::IdleGetter(const Napi::CallbackInfo& info) {
Expand Down
14 changes: 9 additions & 5 deletions src/backup.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ class Backup : public Napi::ObjectWrap<Backup> {

struct InitializeBaton : Database::Baton {
Backup* backup;
Database* otherDb;
std::string filename;
std::string sourceName;
std::string destName;
Expand Down Expand Up @@ -145,11 +146,12 @@ class Backup : public Napi::ObjectWrap<Backup> {
Baton* baton;
};

void init(Database* db_) {
void init(Database* db_, Database* otherDb_) {
db = db_;
otherDb = otherDb_;
_handle = NULL;
_otherDb = NULL;
_destDb = NULL;
_otherDbHandle = NULL;
_destDbHandle = NULL;
inited = false;
locked = true;
completed = false;
Expand Down Expand Up @@ -183,6 +185,7 @@ class Backup : public Napi::ObjectWrap<Backup> {
void RetryErrorSetter(const Napi::CallbackInfo& info, const Napi::Value& value);

protected:
static void Work_BeforeInitialize(Database::Baton* baton);
static void Work_BeginInitialize(Database::Baton* baton);
static void Work_Initialize(napi_env env, void* data);
static void Work_AfterInitialize(napi_env env, napi_status status, void* data);
Expand All @@ -197,10 +200,11 @@ class Backup : public Napi::ObjectWrap<Backup> {
void GetRetryErrors(std::set<int>& retryErrorsSet);

Database* db;
Database* otherDb;

sqlite3_backup* _handle;
sqlite3* _otherDb;
sqlite3* _destDb;
sqlite3* _otherDbHandle;
sqlite3* _destDbHandle;
int status;
std::string message;

Expand Down
51 changes: 51 additions & 0 deletions test/backup.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,4 +276,55 @@ describe('backup', function() {
});
});
});

it ('can backup between two sqlite3.Database instances', function(done) {
var src = new sqlite3.Database(':memory:', function(err) {
if (err) throw err;
src.exec("CREATE TABLE space (txt TEXT)", function(err) {
if (err) throw err;
src.exec("INSERT INTO space(txt) VALUES('monkey')", function(err) {
if (err) throw err;
var dest = new sqlite3.Database(':memory:', function(err) {
if (err) throw err;
var backup = src.backup(dest);
backup.step(-1);
backup.finish(function(err) {
if (err) throw err;
assertRowsMatchDb(src, 'space', dest, 'space', done);
});
});
});
});
});
});

it ('locks destination when backing up between two sqlite3.Database instances', function(done) {
var src = new sqlite3.Database(':memory:', function(err) {
if (err) throw err;
src.exec("CREATE TABLE space (txt TEXT)", function(err) {
if (err) throw err;
src.exec("INSERT INTO space(txt) VALUES('monkey')", function(err) {
if (err) throw err;
var dest = new sqlite3.Database(':memory:', function(err) {
if (err) throw err;
var backup = src.backup(dest, function(err) {
if (err) throw err;
var finished = false;
// This action on dest db should be held until backup finishes.
dest.exec("CREATE TABLE space2 (txt TEXT)", function(err) {
if (err) throw err;
assert(finished);
done();
});
backup.step(-1);
backup.finish(function(err) {
if (err) throw err;
finished = true;
});
});
});
});
});
});
});
});