From 9af26659884fe182219445c196017ef9960ce1b2 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sat, 20 Oct 2018 19:20:49 +0200 Subject: [PATCH 1/6] handle errors in compactRange test --- test/compact-range-test.js | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/test/compact-range-test.js b/test/compact-range-test.js index 2cefb9e0..2482fa96 100644 --- a/test/compact-range-test.js +++ b/test/compact-range-test.js @@ -15,16 +15,32 @@ test('test compactRange() frees disk space after key deletion', function (t) { var key2 = '000001' var val1 = Buffer.allocUnsafe(64).fill(1) var val2 = Buffer.allocUnsafe(64).fill(1) - db.put(key1, val1, function () { - db.put(key2, val2, function () { - db.compactRange(key1, key2, function () { + + // TODO: use batch() + db.put(key1, val1, function (err) { + t.ifError(err, 'no put1 error') + + db.put(key2, val2, function (err) { + t.ifError(err, 'no put2 error') + + db.compactRange(key1, key2, function (err) { + t.ifError(err, 'no compactRange1 error') + db.approximateSize('0', 'z', function (err, sizeAfterPuts) { - t.error(err, 'no error') - db.del(key1, function () { - db.del(key2, function () { - db.compactRange(key1, key2, function () { + t.error(err, 'no approximateSize1 error') + + // TODO: use batch() + db.del(key1, function (err) { + t.ifError(err, 'no del1 error') + + db.del(key2, function (err) { + t.ifError(err, 'no del2 error') + + db.compactRange(key1, key2, function (err) { + t.ifError(err, 'no compactRange2 error') + db.approximateSize('0', 'z', function (err, sizeAfterCompact) { - t.error(err, 'no error') + t.error(err, 'no approximateSize2 error') t.ok(sizeAfterCompact < sizeAfterPuts) t.end() }) From 758a25705159911bc8826bfe4ad61b33cdaf51b7 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sat, 20 Oct 2018 19:25:16 +0200 Subject: [PATCH 2/6] use batch() in compactRange test --- test/compact-range-test.js | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/test/compact-range-test.js b/test/compact-range-test.js index 2482fa96..2c0e6815 100644 --- a/test/compact-range-test.js +++ b/test/compact-range-test.js @@ -16,35 +16,25 @@ test('test compactRange() frees disk space after key deletion', function (t) { var val1 = Buffer.allocUnsafe(64).fill(1) var val2 = Buffer.allocUnsafe(64).fill(1) - // TODO: use batch() - db.put(key1, val1, function (err) { - t.ifError(err, 'no put1 error') + db.batch().put(key1, val1).put(key2, val2).write(function (err) { + t.ifError(err, 'no batch put error') - db.put(key2, val2, function (err) { - t.ifError(err, 'no put2 error') + db.compactRange(key1, key2, function (err) { + t.ifError(err, 'no compactRange1 error') - db.compactRange(key1, key2, function (err) { - t.ifError(err, 'no compactRange1 error') + db.approximateSize('0', 'z', function (err, sizeAfterPuts) { + t.error(err, 'no approximateSize1 error') - db.approximateSize('0', 'z', function (err, sizeAfterPuts) { - t.error(err, 'no approximateSize1 error') + db.batch().del(key1).del(key2).write(function (err) { + t.ifError(err, 'no batch del error') - // TODO: use batch() - db.del(key1, function (err) { - t.ifError(err, 'no del1 error') + db.compactRange(key1, key2, function (err) { + t.ifError(err, 'no compactRange2 error') - db.del(key2, function (err) { - t.ifError(err, 'no del2 error') - - db.compactRange(key1, key2, function (err) { - t.ifError(err, 'no compactRange2 error') - - db.approximateSize('0', 'z', function (err, sizeAfterCompact) { - t.error(err, 'no approximateSize2 error') - t.ok(sizeAfterCompact < sizeAfterPuts) - t.end() - }) - }) + db.approximateSize('0', 'z', function (err, sizeAfterCompact) { + t.error(err, 'no approximateSize2 error') + t.ok(sizeAfterCompact < sizeAfterPuts) + t.end() }) }) }) From 0e3cda49677cae09f709cc74f951c909d5fdea8a Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sat, 20 Oct 2018 21:03:05 +0200 Subject: [PATCH 3/6] serialize compactRange() start and end --- leveldown.js | 3 +++ test/compact-range-test.js | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/leveldown.js b/leveldown.js index 43c82dcf..ee3e4d82 100644 --- a/leveldown.js +++ b/leveldown.js @@ -76,6 +76,9 @@ LevelDOWN.prototype.approximateSize = function (start, end, callback) { } LevelDOWN.prototype.compactRange = function (start, end, callback) { + start = this._serializeKey(start) + end = this._serializeKey(end) + this.binding.compactRange(start, end, callback) } diff --git a/test/compact-range-test.js b/test/compact-range-test.js index 2c0e6815..f4fe4d57 100644 --- a/test/compact-range-test.js +++ b/test/compact-range-test.js @@ -43,6 +43,22 @@ test('test compactRange() frees disk space after key deletion', function (t) { }) }) +test('test compactRange() serializes start and end', function (t) { + t.plan(3) + + var clone = Object.create(db) + var count = 0 + + clone._serializeKey = function (key) { + t.is(key, count++) + return db._serializeKey(key) + } + + clone.compactRange(0, 1, function (err) { + t.ifError(err, 'no compactRange error') + }) +}) + test('tearDown', function (t) { db.close(testCommon.tearDown.bind(null, t)) }) From d54db39fcd386b354bf7164816ec31bf3d8d1ca1 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sun, 21 Oct 2018 09:01:42 +0200 Subject: [PATCH 4/6] support compactRange on full or partial range --- README.md | 10 ++- leveldown.js | 12 +++ src/database_async.cc | 6 +- test/compact-range-test.js | 149 +++++++++++++++++++++++++++++++------ 4 files changed, 151 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 3c5da2e0..47165317 100644 --- a/README.md +++ b/README.md @@ -174,10 +174,16 @@ The `start` and `end` parameters may be strings or Buffers representing keys in The `callback` function will be called with a single `error` if the operation failed for any reason. If successful the first argument will be `null` and the second argument will be the approximate size as a Number. -### `db.compactRange(start, end, callback)` +### `db.compactRange([start][, end], callback)` compactRange() is an instance method on an existing database object. Used to manually trigger a database compaction in the range `[start..end)`. -The `start` and `end` parameters may be strings or Buffers representing keys in the LevelDB store. +The `start` and `end` parameters may be strings or Buffers representing keys in the LevelDB store. If `start` is a zero-length string, a zero-length Buffer or omitted, it is treated as a key *before* all keys in the database. Likewise, if `end` meets those criteria, it is treated as a key *after* all keys in the database. Therefore the following call will compact the entire database: + +```js +db.compactRange(callback) +``` + +Any other type of `start` or `end` will be converted to a string. The `callback` function will be called with no arguments if the operation is successful or with a single `error` argument if the operation failed for any reason. diff --git a/leveldown.js b/leveldown.js index ee3e4d82..099e3e24 100644 --- a/leveldown.js +++ b/leveldown.js @@ -76,6 +76,18 @@ LevelDOWN.prototype.approximateSize = function (start, end, callback) { } LevelDOWN.prototype.compactRange = function (start, end, callback) { + // TODO: consider an options object instead. + if (typeof start === 'function') { + callback = start + start = '' + end = '' + } else if (typeof end === 'function') { + callback = end + end = '' + } else if (typeof callback !== 'function') { + throw new Error('compactRange() requires a callback argument') + } + start = this._serializeKey(start) end = this._serializeKey(end) diff --git a/src/database_async.cc b/src/database_async.cc index ef044e42..fa16d311 100644 --- a/src/database_async.cc +++ b/src/database_async.cc @@ -267,7 +267,11 @@ CompactRangeWorker::CompactRangeWorker(Database *database, }; void CompactRangeWorker::Execute() { - database->CompactRangeFromDatabase(&rangeStart, &rangeEnd); + // TODO: use null as "not defined" signal in JS-land too. + database->CompactRangeFromDatabase( + rangeStart.empty() ? NULL : &rangeStart, + rangeEnd.empty() ? NULL : &rangeEnd + ); } void CompactRangeWorker::WorkComplete() { diff --git a/test/compact-range-test.js b/test/compact-range-test.js index f4fe4d57..b721206d 100644 --- a/test/compact-range-test.js +++ b/test/compact-range-test.js @@ -1,21 +1,14 @@ const test = require('tape') const testCommon = require('./common') -let db +const key1 = '000000' +const key2 = '000001' +const val1 = Buffer.allocUnsafe(64).fill(1) +const val2 = Buffer.allocUnsafe(64).fill(1) -test('setUp common', testCommon.setUp) - -test('setUp db', function (t) { - db = testCommon.factory() - db.open(t.end.bind(t)) -}) - -test('test compactRange() frees disk space after key deletion', function (t) { - var key1 = '000000' - var key2 = '000001' - var val1 = Buffer.allocUnsafe(64).fill(1) - var val2 = Buffer.allocUnsafe(64).fill(1) +test('setUp', testCommon.setUp) +make('test compactRange() frees disk space after key deletion', function (db, t, done) { db.batch().put(key1, val1).put(key2, val2).write(function (err) { t.ifError(err, 'no batch put error') @@ -34,7 +27,7 @@ test('test compactRange() frees disk space after key deletion', function (t) { db.approximateSize('0', 'z', function (err, sizeAfterCompact) { t.error(err, 'no approximateSize2 error') t.ok(sizeAfterCompact < sizeAfterPuts) - t.end() + done() }) }) }) @@ -43,22 +36,132 @@ test('test compactRange() frees disk space after key deletion', function (t) { }) }) -test('test compactRange() serializes start and end', function (t) { - t.plan(3) - - var clone = Object.create(db) +make('test compactRange() serializes start and end', function (db, t, done) { var count = 0 - clone._serializeKey = function (key) { + db._serializeKey = function (key) { t.is(key, count++) - return db._serializeKey(key) + return String(key) } - clone.compactRange(0, 1, function (err) { + db.compactRange(0, 1, function (err) { t.ifError(err, 'no compactRange error') + t.is(count, 2, '_serializeKey called twice') + + done() + }) +}) + +make('test compactRange() throws if callback is missing', function (db, t, done) { + try { + db.compactRange() + } catch (err) { + t.is(err.message, 'compactRange() requires a callback argument') + done() + } +}) + +make('test compactRange() without end', function (db, t, done) { + db.batch().put(key1, val1).put(key2, val2).write(function (err) { + t.ifError(err, 'no batch put error') + + db.compactRange(key1, function (err) { + t.ifError(err, 'no compactRange1 error') + + db.approximateSize('0', 'z', function (err, sizeAfterPuts) { + t.error(err, 'no approximateSize1 error') + + db.batch().del(key1).del(key2).write(function (err) { + t.ifError(err, 'no batch del error') + + db.compactRange(key1, function (err) { + t.ifError(err, 'no compactRange2 error') + + db.approximateSize('0', 'z', function (err, sizeAfterCompact) { + t.error(err, 'no approximateSize2 error') + t.ok(sizeAfterCompact < sizeAfterPuts) + done() + }) + }) + }) + }) + }) + }) +}) + +make('test compactRange() without start and end', function (db, t, done) { + db.batch().put(key1, val1).put(key2, val2).write(function (err) { + t.ifError(err, 'no batch put error') + + db.compactRange(function (err) { + t.ifError(err, 'no compactRange1 error') + + db.approximateSize('0', 'z', function (err, sizeAfterPuts) { + t.error(err, 'no approximateSize1 error') + + db.batch().del(key1).del(key2).write(function (err) { + t.ifError(err, 'no batch del error') + + db.compactRange(function (err) { + t.ifError(err, 'no compactRange2 error') + + db.approximateSize('0', 'z', function (err, sizeAfterCompact) { + t.error(err, 'no approximateSize2 error') + t.ok(sizeAfterCompact < sizeAfterPuts) + done() + }) + }) + }) + }) + }) }) }) -test('tearDown', function (t) { - db.close(testCommon.tearDown.bind(null, t)) +make('test compactRange() outside of key space', function (db, t, done) { + db.batch().put(key1, val1).put(key2, val2).write(function (err) { + t.ifError(err, 'no batch put error') + + db.compactRange(function (err) { + t.ifError(err, 'no compactRange1 error') + + db.approximateSize('0', 'z', function (err, sizeAfterPuts) { + t.error(err, 'no approximateSize1 error') + + db.batch().del(key1).del(key2).write(function (err) { + t.ifError(err, 'no batch del error') + + db.compactRange('z', function (err) { + t.ifError(err, 'no compactRange2 error') + + db.approximateSize('0', 'z', function (err, sizeAfterCompact) { + t.error(err, 'no approximateSize2 error') + t.ok(sizeAfterCompact >= sizeAfterPuts, 'compactRange did nothing') + done() + }) + }) + }) + }) + }) + }) }) + +test('tearDown', testCommon.tearDown) + +function make (name, testFn) { + test(name, function (t) { + var db = testCommon.factory() + var done = function (err) { + t.ifError(err, 'no done error') + + db.close(function (err) { + t.ifError(err, 'no error from close()') + t.end() + }) + } + + db.open(function (err) { + t.ifError(err, 'no error from open()') + testFn(db, t, done) + }) + }) +} From f9a04c0bc281056d40c77678fc5e037248def156 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sat, 24 Nov 2018 13:48:19 +0100 Subject: [PATCH 5/6] Revert "support compactRange on full or partial range" This reverts commit d54db39fcd386b354bf7164816ec31bf3d8d1ca1. --- README.md | 10 +-- leveldown.js | 12 --- src/database_async.cc | 6 +- test/compact-range-test.js | 149 ++++++------------------------------- 4 files changed, 26 insertions(+), 151 deletions(-) diff --git a/README.md b/README.md index 47165317..3c5da2e0 100644 --- a/README.md +++ b/README.md @@ -174,16 +174,10 @@ The `start` and `end` parameters may be strings or Buffers representing keys in The `callback` function will be called with a single `error` if the operation failed for any reason. If successful the first argument will be `null` and the second argument will be the approximate size as a Number. -### `db.compactRange([start][, end], callback)` +### `db.compactRange(start, end, callback)` compactRange() is an instance method on an existing database object. Used to manually trigger a database compaction in the range `[start..end)`. -The `start` and `end` parameters may be strings or Buffers representing keys in the LevelDB store. If `start` is a zero-length string, a zero-length Buffer or omitted, it is treated as a key *before* all keys in the database. Likewise, if `end` meets those criteria, it is treated as a key *after* all keys in the database. Therefore the following call will compact the entire database: - -```js -db.compactRange(callback) -``` - -Any other type of `start` or `end` will be converted to a string. +The `start` and `end` parameters may be strings or Buffers representing keys in the LevelDB store. The `callback` function will be called with no arguments if the operation is successful or with a single `error` argument if the operation failed for any reason. diff --git a/leveldown.js b/leveldown.js index 099e3e24..ee3e4d82 100644 --- a/leveldown.js +++ b/leveldown.js @@ -76,18 +76,6 @@ LevelDOWN.prototype.approximateSize = function (start, end, callback) { } LevelDOWN.prototype.compactRange = function (start, end, callback) { - // TODO: consider an options object instead. - if (typeof start === 'function') { - callback = start - start = '' - end = '' - } else if (typeof end === 'function') { - callback = end - end = '' - } else if (typeof callback !== 'function') { - throw new Error('compactRange() requires a callback argument') - } - start = this._serializeKey(start) end = this._serializeKey(end) diff --git a/src/database_async.cc b/src/database_async.cc index fa16d311..ef044e42 100644 --- a/src/database_async.cc +++ b/src/database_async.cc @@ -267,11 +267,7 @@ CompactRangeWorker::CompactRangeWorker(Database *database, }; void CompactRangeWorker::Execute() { - // TODO: use null as "not defined" signal in JS-land too. - database->CompactRangeFromDatabase( - rangeStart.empty() ? NULL : &rangeStart, - rangeEnd.empty() ? NULL : &rangeEnd - ); + database->CompactRangeFromDatabase(&rangeStart, &rangeEnd); } void CompactRangeWorker::WorkComplete() { diff --git a/test/compact-range-test.js b/test/compact-range-test.js index b721206d..f4fe4d57 100644 --- a/test/compact-range-test.js +++ b/test/compact-range-test.js @@ -1,14 +1,21 @@ const test = require('tape') const testCommon = require('./common') -const key1 = '000000' -const key2 = '000001' -const val1 = Buffer.allocUnsafe(64).fill(1) -const val2 = Buffer.allocUnsafe(64).fill(1) +let db -test('setUp', testCommon.setUp) +test('setUp common', testCommon.setUp) + +test('setUp db', function (t) { + db = testCommon.factory() + db.open(t.end.bind(t)) +}) + +test('test compactRange() frees disk space after key deletion', function (t) { + var key1 = '000000' + var key2 = '000001' + var val1 = Buffer.allocUnsafe(64).fill(1) + var val2 = Buffer.allocUnsafe(64).fill(1) -make('test compactRange() frees disk space after key deletion', function (db, t, done) { db.batch().put(key1, val1).put(key2, val2).write(function (err) { t.ifError(err, 'no batch put error') @@ -27,7 +34,7 @@ make('test compactRange() frees disk space after key deletion', function (db, t, db.approximateSize('0', 'z', function (err, sizeAfterCompact) { t.error(err, 'no approximateSize2 error') t.ok(sizeAfterCompact < sizeAfterPuts) - done() + t.end() }) }) }) @@ -36,132 +43,22 @@ make('test compactRange() frees disk space after key deletion', function (db, t, }) }) -make('test compactRange() serializes start and end', function (db, t, done) { +test('test compactRange() serializes start and end', function (t) { + t.plan(3) + + var clone = Object.create(db) var count = 0 - db._serializeKey = function (key) { + clone._serializeKey = function (key) { t.is(key, count++) - return String(key) + return db._serializeKey(key) } - db.compactRange(0, 1, function (err) { + clone.compactRange(0, 1, function (err) { t.ifError(err, 'no compactRange error') - t.is(count, 2, '_serializeKey called twice') - - done() - }) -}) - -make('test compactRange() throws if callback is missing', function (db, t, done) { - try { - db.compactRange() - } catch (err) { - t.is(err.message, 'compactRange() requires a callback argument') - done() - } -}) - -make('test compactRange() without end', function (db, t, done) { - db.batch().put(key1, val1).put(key2, val2).write(function (err) { - t.ifError(err, 'no batch put error') - - db.compactRange(key1, function (err) { - t.ifError(err, 'no compactRange1 error') - - db.approximateSize('0', 'z', function (err, sizeAfterPuts) { - t.error(err, 'no approximateSize1 error') - - db.batch().del(key1).del(key2).write(function (err) { - t.ifError(err, 'no batch del error') - - db.compactRange(key1, function (err) { - t.ifError(err, 'no compactRange2 error') - - db.approximateSize('0', 'z', function (err, sizeAfterCompact) { - t.error(err, 'no approximateSize2 error') - t.ok(sizeAfterCompact < sizeAfterPuts) - done() - }) - }) - }) - }) - }) - }) -}) - -make('test compactRange() without start and end', function (db, t, done) { - db.batch().put(key1, val1).put(key2, val2).write(function (err) { - t.ifError(err, 'no batch put error') - - db.compactRange(function (err) { - t.ifError(err, 'no compactRange1 error') - - db.approximateSize('0', 'z', function (err, sizeAfterPuts) { - t.error(err, 'no approximateSize1 error') - - db.batch().del(key1).del(key2).write(function (err) { - t.ifError(err, 'no batch del error') - - db.compactRange(function (err) { - t.ifError(err, 'no compactRange2 error') - - db.approximateSize('0', 'z', function (err, sizeAfterCompact) { - t.error(err, 'no approximateSize2 error') - t.ok(sizeAfterCompact < sizeAfterPuts) - done() - }) - }) - }) - }) - }) }) }) -make('test compactRange() outside of key space', function (db, t, done) { - db.batch().put(key1, val1).put(key2, val2).write(function (err) { - t.ifError(err, 'no batch put error') - - db.compactRange(function (err) { - t.ifError(err, 'no compactRange1 error') - - db.approximateSize('0', 'z', function (err, sizeAfterPuts) { - t.error(err, 'no approximateSize1 error') - - db.batch().del(key1).del(key2).write(function (err) { - t.ifError(err, 'no batch del error') - - db.compactRange('z', function (err) { - t.ifError(err, 'no compactRange2 error') - - db.approximateSize('0', 'z', function (err, sizeAfterCompact) { - t.error(err, 'no approximateSize2 error') - t.ok(sizeAfterCompact >= sizeAfterPuts, 'compactRange did nothing') - done() - }) - }) - }) - }) - }) - }) +test('tearDown', function (t) { + db.close(testCommon.tearDown.bind(null, t)) }) - -test('tearDown', testCommon.tearDown) - -function make (name, testFn) { - test(name, function (t) { - var db = testCommon.factory() - var done = function (err) { - t.ifError(err, 'no done error') - - db.close(function (err) { - t.ifError(err, 'no error from close()') - t.end() - }) - } - - db.open(function (err) { - t.ifError(err, 'no error from open()') - testFn(db, t, done) - }) - }) -} From 2c4a2094b1f70e40c2b856ffd0997a0cc59e90dc Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Sat, 24 Nov 2018 13:58:32 +0100 Subject: [PATCH 6/6] copy type checks from approximateSize to compactRange --- leveldown.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/leveldown.js b/leveldown.js index ee3e4d82..7e09e73c 100644 --- a/leveldown.js +++ b/leveldown.js @@ -62,7 +62,7 @@ LevelDOWN.prototype.approximateSize = function (start, end, callback) { end == null || typeof start === 'function' || typeof end === 'function') { - throw new Error('approximateSize() requires valid `start`, `end` and `callback` arguments') + throw new Error('approximateSize() requires valid `start` and `end` arguments') } if (typeof callback !== 'function') { @@ -76,6 +76,17 @@ LevelDOWN.prototype.approximateSize = function (start, end, callback) { } LevelDOWN.prototype.compactRange = function (start, end, callback) { + if (start == null || + end == null || + typeof start === 'function' || + typeof end === 'function') { + throw new Error('compactRange() requires valid `start` and `end` arguments') + } + + if (typeof callback !== 'function') { + throw new Error('compactRange() requires a callback argument') + } + start = this._serializeKey(start) end = this._serializeKey(end)