From 5b9a12256c9907e7bfba633d39c1c851fb78a2ae Mon Sep 17 00:00:00 2001 From: Eugene Ware Date: Thu, 4 Apr 2019 19:59:12 +1100 Subject: [PATCH 1/5] Added `readOnly` rocksdb option --- src/database.cc | 11 +++++++++-- src/database.h | 2 +- src/database_async.cc | 4 +++- src/database_async.h | 2 ++ 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/database.cc b/src/database.cc index c93b3b58..5c11c5fc 100644 --- a/src/database.cc +++ b/src/database.cc @@ -33,9 +33,14 @@ Database::~Database () { /* Calls from worker threads, NO V8 HERE *****************************/ rocksdb::Status Database::OpenDatabase ( - rocksdb::Options* options + rocksdb::Options* options, + bool readOnly ) { - return rocksdb::DB::Open(*options, **location, &db); + if (readOnly) { + return rocksdb::DB::OpenForReadOnly(*options, **location, &db); + } else { + return rocksdb::DB::Open(*options, **location, &db); + } } rocksdb::Status Database::PutToDatabase ( @@ -181,6 +186,7 @@ v8::Local Database::NewInstance (v8::Local &location) { NAN_METHOD(Database::Open) { LD_METHOD_SETUP_COMMON(open, 0, 1) + bool readOnly = BooleanOptionValue(optionsObj, "readOnly", false); bool createIfMissing = BooleanOptionValue(optionsObj, "createIfMissing", true); bool errorIfExists = BooleanOptionValue(optionsObj, "errorIfExists"); bool compression = BooleanOptionValue(optionsObj, "compression", true); @@ -217,6 +223,7 @@ NAN_METHOD(Database::Open) { , maxOpenFiles , blockRestartInterval , maxFileSize + , readOnly ); // persist to prevent accidental GC v8::Local _this = info.This(); diff --git a/src/database.h b/src/database.h index cd2f83df..1beaa03a 100644 --- a/src/database.h +++ b/src/database.h @@ -43,7 +43,7 @@ class Database : public Nan::ObjectWrap { static void Init (); static v8::Local NewInstance (v8::Local &location); - rocksdb::Status OpenDatabase (rocksdb::Options* options); + rocksdb::Status OpenDatabase (rocksdb::Options* options, bool readOnly); rocksdb::Status PutToDatabase ( rocksdb::WriteOptions* options , rocksdb::Slice key diff --git a/src/database_async.cc b/src/database_async.cc index 8464c530..c24c6c17 100644 --- a/src/database_async.cc +++ b/src/database_async.cc @@ -36,7 +36,9 @@ OpenWorker::OpenWorker ( , uint32_t maxOpenFiles , uint32_t blockRestartInterval , uint32_t maxFileSize + , bool readOnly ) : AsyncWorker(database, callback, "rocksdb:db.open") + , readOnly_(readOnly) { rocksdb::LevelDBOptions levelOptions; @@ -85,7 +87,7 @@ OpenWorker::~OpenWorker () { } void OpenWorker::Execute () { - SetStatus(database->OpenDatabase(options)); + SetStatus(database->OpenDatabase(options, readOnly_)); } /** CLOSE WORKER **/ diff --git a/src/database_async.h b/src/database_async.h index 236a58e3..61e6c438 100644 --- a/src/database_async.h +++ b/src/database_async.h @@ -25,6 +25,7 @@ class OpenWorker : public AsyncWorker { , uint32_t maxOpenFiles , uint32_t blockRestartInterval , uint32_t maxFileSize + , bool readOnly ); virtual ~OpenWorker (); @@ -32,6 +33,7 @@ class OpenWorker : public AsyncWorker { private: rocksdb::Options* options; + bool readOnly_; }; class CloseWorker : public AsyncWorker { From 813246282ab2caa4141ded1f4366842b6cd62d2f Mon Sep 17 00:00:00 2001 From: Eugene Ware Date: Fri, 5 Apr 2019 01:21:43 +1100 Subject: [PATCH 2/5] Add tests for read only rocksdb db --- test/open-read-only-test.js | 123 ++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 test/open-read-only-test.js diff --git a/test/open-read-only-test.js b/test/open-read-only-test.js new file mode 100644 index 00000000..e1b830ec --- /dev/null +++ b/test/open-read-only-test.js @@ -0,0 +1,123 @@ +'use strict' + +const test = require('tape') +const leveldown = require('../') +const testCommon = require('abstract-leveldown/testCommon') +const fs = require('fs') +var location = testCommon.location() + +test('setUp', function (t) { + // just in case we thew an error last time and don't have perms to remove db + if (fs.existsSync(location)) { + fs.chmodSync(location, 0o755) + } + testCommon.setUp(t) +}) + +test('test write to read/write database', function (t) { + var db = leveldown(location) + db.open(function (err) { + t.error(err) + db.put('my key', 'my value', function (err) { + t.error(err, 'no write error') + db.get('my key', function (err, value) { + t.error(err, 'no read error') + t.equal(value.toString(), 'my value', 'correct value') + db.close(t.end.bind(t)) + }) + }) + }) +}) + +test('test throw error reading read-only database', function (t) { + fs.chmodSync(location, 0o555) + var db = leveldown(location) + db.open(function (err) { + t.ok(err, 'should get error reading read only database') + t.ok(/IO Error/i.test(err && err.message), 'should get io error') + db.close(t.end.bind(t)) + }) +}) + +test('test read from a read-only database if readOnly is true', function (t) { + fs.chmodSync(location, 0o555) + var db = leveldown(location) + db.open({ readOnly: true }, function (err) { + t.error(err) + db.get('my key', function (err, value) { + t.error(err, 'no read error') + t.equal(value.toString(), 'my value', 'correct value') + db.close(t.end.bind(t)) + }) + }) +}) + +test('test throw error reading read-only database if readOnly is false', function (t) { + fs.chmodSync(location, 0o555) + var db = leveldown(location) + db.open({ readOnly: false }, function (err) { + t.ok(err, 'should get error reading read only database') + t.ok(/IO Error/i.test(err && err.message), 'should get io error') + db.close(t.end.bind(t)) + }) +}) + +test('test throw error putting data to read-only db if readOnly is true', function (t) { + fs.chmodSync(location, 0o555) + var db = leveldown(location) + db.open({ readOnly: true }, function (err) { + t.error(err) + db.put('my key', 'my value', function (err) { + t.ok(err, 'should get write error') + t.ok(/Not supported operation in read only mode/i.test(err && err.message), 'should get io error') + db.close(t.end.bind(t)) + }) + }) +}) + +test('test throw error deleting data from read-only db if readOnly is true', function (t) { + fs.chmodSync(location, 0o555) + var db = leveldown(location) + db.open({ readOnly: true }, function (err) { + t.error(err) + db.del('my key', function (err) { + t.ok(err, 'should get write error') + t.ok(/Not supported operation in read only mode/i.test(err && err.message), 'should get io error') + db.close(t.end.bind(t)) + }) + }) +}) + +test('test throw error with batch put from read-only db if readOnly is true', function (t) { + fs.chmodSync(location, 0o555) + var db = leveldown(location) + db.open({ readOnly: true }, function (err) { + t.error(err) + db.batch([{ key: 'key 1', type: 'put', value: 'value 1' }], function (err) { + t.ok(err, 'should get write error') + t.ok(/Not supported operation in read only mode/i.test(err && err.message), 'should get io error') + db.close(t.end.bind(t)) + }) + }) +}) + +test('test throw error with batch del from read-only db if readOnly is true', function (t) { + fs.chmodSync(location, 0o555) + var db = leveldown(location) + db.open({ readOnly: true }, function (err) { + t.error(err) + db.batch([{ key: 'my key', type: 'del' }], function (err) { + t.ok(err, 'should get write error') + t.ok(/Not supported operation in read only mode/i.test(err && err.message), 'should get io error') + db.close(t.end.bind(t)) + }) + }) +}) + +test('tearDown', function (t) { + // just in case we thew an error last time and don't have perms to remove db + if (fs.existsSync(location)) { + fs.chmodSync(location, 0o755) + } + testCommon.tearDown(t) +}) From e643c6737d85cc6520e28b671efb636a8f45f08e Mon Sep 17 00:00:00 2001 From: Eugene Ware Date: Fri, 5 Apr 2019 06:58:00 -0700 Subject: [PATCH 3/5] Make database files read only for windows Making a parent directory read only doesn't cascade to the files in that folder like in *nix --- test/open-read-only-test.js | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/open-read-only-test.js b/test/open-read-only-test.js index e1b830ec..572635f2 100644 --- a/test/open-read-only-test.js +++ b/test/open-read-only-test.js @@ -4,8 +4,18 @@ const test = require('tape') const leveldown = require('../') const testCommon = require('abstract-leveldown/testCommon') const fs = require('fs') +const path = require('path') + var location = testCommon.location() +function chmodFilesSync (dir, mode) { + var files = fs.readdirSync(dir) + files.forEach(function (file) { + var fullPath = path.join(dir, file) + fs.chmodSync(fullPath, mode) + }) +} + test('setUp', function (t) { // just in case we thew an error last time and don't have perms to remove db if (fs.existsSync(location)) { @@ -30,6 +40,7 @@ test('test write to read/write database', function (t) { }) test('test throw error reading read-only database', function (t) { + chmodFilesSync(location, 0o555) fs.chmodSync(location, 0o555) var db = leveldown(location) db.open(function (err) { @@ -40,6 +51,7 @@ test('test throw error reading read-only database', function (t) { }) test('test read from a read-only database if readOnly is true', function (t) { + chmodFilesSync(location, 0o555) fs.chmodSync(location, 0o555) var db = leveldown(location) db.open({ readOnly: true }, function (err) { @@ -53,6 +65,7 @@ test('test read from a read-only database if readOnly is true', function (t) { }) test('test throw error reading read-only database if readOnly is false', function (t) { + chmodFilesSync(location, 0o555) fs.chmodSync(location, 0o555) var db = leveldown(location) db.open({ readOnly: false }, function (err) { @@ -63,6 +76,7 @@ test('test throw error reading read-only database if readOnly is false', functio }) test('test throw error putting data to read-only db if readOnly is true', function (t) { + chmodFilesSync(location, 0o555) fs.chmodSync(location, 0o555) var db = leveldown(location) db.open({ readOnly: true }, function (err) { @@ -76,6 +90,7 @@ test('test throw error putting data to read-only db if readOnly is true', functi }) test('test throw error deleting data from read-only db if readOnly is true', function (t) { + chmodFilesSync(location, 0o555) fs.chmodSync(location, 0o555) var db = leveldown(location) db.open({ readOnly: true }, function (err) { @@ -89,6 +104,7 @@ test('test throw error deleting data from read-only db if readOnly is true', fun }) test('test throw error with batch put from read-only db if readOnly is true', function (t) { + chmodFilesSync(location, 0o555) fs.chmodSync(location, 0o555) var db = leveldown(location) db.open({ readOnly: true }, function (err) { @@ -102,6 +118,7 @@ test('test throw error with batch put from read-only db if readOnly is true', fu }) test('test throw error with batch del from read-only db if readOnly is true', function (t) { + chmodFilesSync(location, 0o555) fs.chmodSync(location, 0o555) var db = leveldown(location) db.open({ readOnly: true }, function (err) { From 3074a41980a15145d846bf83d05a82fb86b510ad Mon Sep 17 00:00:00 2001 From: Eugene Ware Date: Fri, 5 Apr 2019 07:00:27 -0700 Subject: [PATCH 4/5] Remove batch operations. Not really testing anything. --- test/open-read-only-test.js | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/test/open-read-only-test.js b/test/open-read-only-test.js index 572635f2..1233fa5a 100644 --- a/test/open-read-only-test.js +++ b/test/open-read-only-test.js @@ -103,34 +103,6 @@ test('test throw error deleting data from read-only db if readOnly is true', fun }) }) -test('test throw error with batch put from read-only db if readOnly is true', function (t) { - chmodFilesSync(location, 0o555) - fs.chmodSync(location, 0o555) - var db = leveldown(location) - db.open({ readOnly: true }, function (err) { - t.error(err) - db.batch([{ key: 'key 1', type: 'put', value: 'value 1' }], function (err) { - t.ok(err, 'should get write error') - t.ok(/Not supported operation in read only mode/i.test(err && err.message), 'should get io error') - db.close(t.end.bind(t)) - }) - }) -}) - -test('test throw error with batch del from read-only db if readOnly is true', function (t) { - chmodFilesSync(location, 0o555) - fs.chmodSync(location, 0o555) - var db = leveldown(location) - db.open({ readOnly: true }, function (err) { - t.error(err) - db.batch([{ key: 'my key', type: 'del' }], function (err) { - t.ok(err, 'should get write error') - t.ok(/Not supported operation in read only mode/i.test(err && err.message), 'should get io error') - db.close(t.end.bind(t)) - }) - }) -}) - test('tearDown', function (t) { // just in case we thew an error last time and don't have perms to remove db if (fs.existsSync(location)) { From b8b5d9089c967b5d9911bd7b9d2f9c6bc0bcc448 Mon Sep 17 00:00:00 2001 From: Eugene Ware Date: Fri, 5 Apr 2019 07:09:41 -0700 Subject: [PATCH 5/5] Make files writable again for windows to cleanup folder --- test/open-read-only-test.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/open-read-only-test.js b/test/open-read-only-test.js index 1233fa5a..c05ed5e8 100644 --- a/test/open-read-only-test.js +++ b/test/open-read-only-test.js @@ -8,6 +8,7 @@ const path = require('path') var location = testCommon.location() +// This is used because it's not sufficient on windows to set a parent folder as readonly function chmodFilesSync (dir, mode) { var files = fs.readdirSync(dir) files.forEach(function (file) { @@ -20,6 +21,7 @@ test('setUp', function (t) { // just in case we thew an error last time and don't have perms to remove db if (fs.existsSync(location)) { fs.chmodSync(location, 0o755) + chmodFilesSync(location, 0o755) } testCommon.setUp(t) }) @@ -107,6 +109,7 @@ test('tearDown', function (t) { // just in case we thew an error last time and don't have perms to remove db if (fs.existsSync(location)) { fs.chmodSync(location, 0o755) + chmodFilesSync(location, 0o755) } testCommon.tearDown(t) })