From 002d836b0f0e26c83f81e159dbc6724c597a547a Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Thu, 30 May 2019 09:51:42 +0300 Subject: [PATCH] Disable info log by default and add infoLogLevel option (#114) --- .gitignore | 1 + .npmignore | 1 + README.md | 1 + binding.cc | 44 +++++++++++++++++++++++++++++++++++++++++--- leveldown.js | 24 +----------------------- test/destroy-test.js | 3 ++- 6 files changed, 47 insertions(+), 27 deletions(-) diff --git a/.gitignore b/.gitignore index 36806e2c..89b8b82c 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,4 @@ prebuilds/ yarn.lock package-lock.json .nyc_output/ +.benchmarks/ diff --git a/.npmignore b/.npmignore index 87f53d9a..4d2b85e0 100644 --- a/.npmignore +++ b/.npmignore @@ -42,6 +42,7 @@ libleveldb.a # Benchmarks and tests bench/ test/ +.benchmarks/ *.csv # Misc diff --git a/README.md b/README.md index f9c323f0..896ef1f3 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,7 @@ If you don't want to use the prebuilt binary for the platform you are installing Please refer to [`leveldown`](https://github.com/Level/leveldown#api) for API documentation. The `db.open(options, callback)` method of `rocksdb` has a few additional options: - `readOnly` (boolean, default `false`): open database in read-only mode. +- `infoLogLevel` (string, default `null`): verbosity of info log. One of `'debug'`, `'info'`, `'warn'`, `'error'`, `'fatal'`, `'header'` or `null` (disable). ## Contributing diff --git a/binding.cc b/binding.cc index 45bbeee1..ed0de142 100644 --- a/binding.cc +++ b/binding.cc @@ -8,7 +8,6 @@ #include #include #include -#include #include #include #include @@ -20,6 +19,13 @@ namespace leveldb = rocksdb; #include #include +class NullLogger : public rocksdb::Logger { +public: + using rocksdb::Logger::Logv; + virtual void Logv(const char* format, va_list ap) override {} + virtual size_t GetLogFileSize() const override { return 0; } +}; + /** * Forward declarations. */ @@ -777,6 +783,7 @@ struct OpenWorker final : public BaseWorker { uint32_t blockRestartInterval, uint32_t maxFileSize, uint32_t cacheSize, + const std::string& infoLogLevel, bool readOnly) : BaseWorker(env, database, callback, "leveldown.db.open"), readOnly_(readOnly), @@ -790,7 +797,25 @@ struct OpenWorker final : public BaseWorker { options_.max_open_files = maxOpenFiles; options_.max_log_file_size = maxFileSize; options_.paranoid_checks = false; - options_.info_log_level = rocksdb::InfoLogLevel::WARN_LEVEL; + + if (infoLogLevel.size() > 0) { + rocksdb::InfoLogLevel lvl; + + if (infoLogLevel == "debug") lvl = rocksdb::InfoLogLevel::DEBUG_LEVEL; + else if (infoLogLevel == "info") lvl = rocksdb::InfoLogLevel::INFO_LEVEL; + else if (infoLogLevel == "warn") lvl = rocksdb::InfoLogLevel::WARN_LEVEL; + else if (infoLogLevel == "error") lvl = rocksdb::InfoLogLevel::ERROR_LEVEL; + else if (infoLogLevel == "fatal") lvl = rocksdb::InfoLogLevel::FATAL_LEVEL; + else if (infoLogLevel == "header") lvl = rocksdb::InfoLogLevel::HEADER_LEVEL; + else napi_throw_error(env_, NULL, "invalid log level"); + + options_.info_log_level = lvl; + } else { + // In some places RocksDB checks this option to see if it should prepare + // debug information (ahead of logging), so set it to the highest level. + options_.info_log_level = rocksdb::InfoLogLevel::HEADER_LEVEL; + options_.info_log.reset(new NullLogger()); + } rocksdb::BlockBasedTableOptions tableOptions; @@ -834,6 +859,8 @@ NAPI_METHOD(db_open) { bool compression = BooleanProperty(env, options, "compression", true); bool readOnly = BooleanProperty(env, options, "readOnly", false); + std::string infoLogLevel = StringProperty(env, options, "infoLogLevel"); + uint32_t cacheSize = Uint32Property(env, options, "cacheSize", 8 << 20); uint32_t writeBufferSize = Uint32Property(env, options , "writeBufferSize" , 4 << 20); uint32_t blockSize = Uint32Property(env, options, "blockSize", 4096); @@ -847,7 +874,8 @@ NAPI_METHOD(db_open) { createIfMissing, errorIfExists, compression, writeBufferSize, blockSize, maxOpenFiles, blockRestartInterval, - maxFileSize, cacheSize, readOnly); + maxFileSize, cacheSize, + infoLogLevel, readOnly); worker->Queue(); delete [] location; @@ -1192,6 +1220,11 @@ struct DestroyWorker final : public BaseWorker { void DoExecute () override { leveldb::Options options; + + // TODO: support overriding infoLogLevel the same as db.open(options) + options.info_log_level = rocksdb::InfoLogLevel::HEADER_LEVEL; + options.info_log.reset(new NullLogger()); + SetStatus(leveldb::DestroyDB(location_, options)); } @@ -1228,6 +1261,11 @@ struct RepairWorker final : public BaseWorker { void DoExecute () override { leveldb::Options options; + + // TODO: support overriding infoLogLevel the same as db.open(options) + options.info_log_level = rocksdb::InfoLogLevel::HEADER_LEVEL; + options.info_log.reset(new NullLogger()); + SetStatus(leveldb::RepairDB(location_, options)); } diff --git a/leveldown.js b/leveldown.js index ae378ced..4452190b 100644 --- a/leveldown.js +++ b/leveldown.js @@ -3,7 +3,6 @@ const AbstractLevelDOWN = require('abstract-leveldown').AbstractLevelDOWN const binding = require('./binding') const ChainedBatch = require('./chained-batch') const Iterator = require('./iterator') -const fs = require('fs') function LevelDOWN (location) { if (!(this instanceof LevelDOWN)) { @@ -122,28 +121,7 @@ LevelDOWN.destroy = function (location, callback) { throw new Error('destroy() requires a callback function argument') } - binding.destroy_db(location, function (err) { - if (err) return callback(err) - - // On Windows, RocksDB silently fails to remove the directory because its - // Logger, which is instantiated on destroy(), has an open file handle on a - // LOG file. Destroy() removes this file but Windows won't actually delete - // it until the handle is released. This happens when destroy() goes out of - // scope, which disposes the Logger. So back in JS-land, we can again - // attempt to remove the directory. This is merely a workaround because - // arguably RocksDB should not instantiate a Logger or open a file at all. - fs.rmdir(location, function (err) { - if (err) { - // Ignore this error in case there are non-RocksDB files left. - if (err.code === 'ENOTEMPTY') return callback() - if (err.code === 'ENOENT') return callback() - - return callback(err) - } - - callback() - }) - }) + binding.destroy_db(location, callback) } LevelDOWN.repair = function (location, callback) { diff --git a/test/destroy-test.js b/test/destroy-test.js index 385f6d04..0ca2de71 100644 --- a/test/destroy-test.js +++ b/test/destroy-test.js @@ -38,7 +38,8 @@ test('test destroy non-existent directory', function (t) { t.ifError(err, 'no error from rimraf()') leveldown.destroy(location, function (err) { - t.error(err, 'no error') + // This behavior differs from LevelDB, which is silent. + t.ok(/.*IO error.*/.test(err), 'got IO error') // Assert that destroy() didn't inadvertently create the directory. // Or if it did, that it was at least cleaned up afterwards.