From df3c610d22bb7a9910432ae308d0efcd956582f6 Mon Sep 17 00:00:00 2001 From: web3-developer <51288821+web3-developer@users.noreply.github.com> Date: Sun, 7 Jul 2024 22:29:01 +0800 Subject: [PATCH 1/5] Add support for optimistic transaction db. --- rocksdb/optimistictxdb.nim | 131 ++++++++++++ rocksdb/transactiondb.nim | 4 +- rocksdb/transactions/otxopts.nim | 49 +++++ rocksdb/transactions/transaction.nim | 6 +- tests/test_all.nim | 2 + tests/test_helper.nim | 16 +- tests/test_optimistictxdb.nim | 291 +++++++++++++++++++++++++++ tests/transactions/test_otxopts.nim | 36 ++++ tests/transactions/test_txdbopts.nim | 6 +- tests/transactions/test_txopts.nim | 6 +- 10 files changed, 536 insertions(+), 11 deletions(-) create mode 100644 rocksdb/optimistictxdb.nim create mode 100644 rocksdb/transactions/otxopts.nim create mode 100644 tests/test_optimistictxdb.nim create mode 100644 tests/transactions/test_otxopts.nim diff --git a/rocksdb/optimistictxdb.nim b/rocksdb/optimistictxdb.nim new file mode 100644 index 0000000..e54ff46 --- /dev/null +++ b/rocksdb/optimistictxdb.nim @@ -0,0 +1,131 @@ +# Nim-RocksDB +# Copyright 2024 Status Research & Development GmbH +# Licensed under either of +# +# * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or http://www.apache.org/licenses/LICENSE-2.0) +# * GPL license, version 2.0, ([LICENSE-GPLv2](LICENSE-GPLv2) or https://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html) +# +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +## A `OptimisticTxDbRef` can be used to open a connection to the RocksDB database +## with support for transactional operations against multiple column families. +## To create a new transaction call `beginTransaction` which will return a +## `TransactionRef`. To commit or rollback the transaction call `commit` or +## `rollback` on the `TransactionRef` type after applying changes to the transaction. + +{.push raises: [].} + +import + std/[sequtils, locks], + ./lib/librocksdb, + ./options/[dbopts, readopts, writeopts], + ./transactions/[transaction, otxopts], + ./columnfamily/[cfopts, cfdescriptor, cfhandle], + ./internal/[cftable, utils], + ./rocksresult + +export dbopts, cfdescriptor, readopts, writeopts, otxopts, transaction, rocksresult + +type + OptimisticTxDbPtr* = ptr rocksdb_optimistictransactiondb_t + + OptimisticTxDbRef* = ref object + lock: Lock + cPtr: OptimisticTxDbPtr + path: string + dbOpts: DbOptionsRef + cfDescriptors: seq[ColFamilyDescriptor] + defaultCfHandle: ColFamilyHandleRef + cfTable: ColFamilyTableRef + +proc openOptimisticTxDb*( + path: string, + dbOpts = defaultDbOptions(autoClose = true), + columnFamilies: openArray[ColFamilyDescriptor] = [], +): RocksDBResult[OptimisticTxDbRef] = + ## Open a `OptimisticTxDbRef` with the given options and column families. + ## If no column families are provided the default column family will be used. + ## If no options are provided the default options will be used. + ## These default options will be closed when the database is closed. + ## If any options are provided, they will need to be closed manually. + + var cfs = columnFamilies.toSeq() + if DEFAULT_COLUMN_FAMILY_NAME notin columnFamilies.mapIt(it.name()): + cfs.add(defaultColFamilyDescriptor(autoClose = true)) + + var + cfNames = cfs.mapIt(it.name().cstring) + cfOpts = cfs.mapIt(it.options.cPtr) + cfHandles = newSeq[ColFamilyHandlePtr](cfs.len) + errors: cstring + + let txDbPtr = rocksdb_optimistictransactiondb_open_column_families( + dbOpts.cPtr, + path.cstring, + cfNames.len().cint, + cast[cstringArray](cfNames[0].addr), + cfOpts[0].addr, + cfHandles[0].addr, + cast[cstringArray](errors.addr), + ) + bailOnErrorsWithCleanup(errors): + autoCloseNonNil(dbOpts) + autoCloseAll(cfs) + + let + cfTable = newColFamilyTable(cfNames.mapIt($it), cfHandles) + db = OptimisticTxDbRef( + lock: createLock(), + cPtr: txDbPtr, + path: path, + dbOpts: dbOpts, + cfDescriptors: cfs, + defaultCfHandle: cfTable.get(DEFAULT_COLUMN_FAMILY_NAME), + cfTable: cfTable, + ) + ok(db) + +proc getColFamilyHandle*( + db: OptimisticTxDbRef, name: string +): RocksDBResult[ColFamilyHandleRef] = + let cfHandle = db.cfTable.get(name) + if cfHandle.isNil(): + err("rocksdb: unknown column family") + else: + ok(cfHandle) + +proc isClosed*(db: OptimisticTxDbRef): bool {.inline.} = + ## Returns `true` if the `OptimisticTxDbRef` has been closed. + db.cPtr.isNil() + +proc beginTransaction*( + db: OptimisticTxDbRef, + readOpts = defaultReadOptions(autoClose = true), + writeOpts = defaultWriteOptions(autoClose = true), + otxOpts = defaultOptimisticTxOptions(autoClose = true), + cfHandle = db.defaultCfHandle, +): TransactionRef = + ## Begin a new transaction against the database. The transaction will default + ## to using the specified column family. If no column family is specified + ## then the default column family will be used. + doAssert not db.isClosed() + + let txPtr = + rocksdb_optimistictransaction_begin(db.cPtr, writeOpts.cPtr, otxOpts.cPtr, nil) + + newTransaction(txPtr, readOpts, writeOpts, nil, otxOpts, cfHandle) + +proc close*(db: OptimisticTxDbRef) = + ## Close the `OptimisticTxDbRef`. + + withLock(db.lock): + if not db.isClosed(): + # the column families should be closed before the database + db.cfTable.close() + + rocksdb_optimistictransactiondb_close(db.cPtr) + db.cPtr = nil + + # opts should be closed after the database is closed + autoCloseNonNil(db.dbOpts) + autoCloseAll(db.cfDescriptors) diff --git a/rocksdb/transactiondb.nim b/rocksdb/transactiondb.nim index ba591d5..d3be25a 100644 --- a/rocksdb/transactiondb.nim +++ b/rocksdb/transactiondb.nim @@ -114,13 +114,11 @@ proc beginTransaction*( ## Begin a new transaction against the database. The transaction will default ## to using the specified column family. If no column family is specified ## then the default column family will be used. - ## - ## doAssert not db.isClosed() let txPtr = rocksdb_transaction_begin(db.cPtr, writeOpts.cPtr, txOpts.cPtr, nil) - newTransaction(txPtr, readOpts, writeOpts, txOpts, cfHandle) + newTransaction(txPtr, readOpts, writeOpts, txOpts, nil, cfHandle) proc close*(db: TransactionDbRef) = ## Close the `TransactionDbRef`. diff --git a/rocksdb/transactions/otxopts.nim b/rocksdb/transactions/otxopts.nim new file mode 100644 index 0000000..89a60c3 --- /dev/null +++ b/rocksdb/transactions/otxopts.nim @@ -0,0 +1,49 @@ +# Nim-RocksDB +# Copyright 2024 Status Research & Development GmbH +# Licensed under either of +# +# * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or http://www.apache.org/licenses/LICENSE-2.0) +# * GPL license, version 2.0, ([LICENSE-GPLv2](LICENSE-GPLv2) or https://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html) +# +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +{.push raises: [].} + +import ../lib/librocksdb + +type + OptimisticTxOptionsPtr* = ptr rocksdb_optimistictransaction_options_t + + OptimisticTxOptionsRef* = ref object + cPtr: OptimisticTxOptionsPtr + autoClose*: bool # if true then close will be called when the transaction is closed + +proc createOptimisticTxOptions*(autoClose = false): OptimisticTxOptionsRef = + OptimisticTxOptionsRef( + cPtr: rocksdb_optimistictransaction_options_create(), autoClose: autoClose + ) + +proc isClosed*(txOpts: OptimisticTxOptionsRef): bool {.inline.} = + txOpts.cPtr.isNil() + +proc cPtr*(txOpts: OptimisticTxOptionsRef): OptimisticTxOptionsPtr = + doAssert not txOpts.isClosed() + txOpts.cPtr + +template setOpt(nname, ntyp, ctyp: untyped) = + proc `nname=`*(txOpts: OptimisticTxOptionsRef, value: ntyp) = + doAssert not txOpts.isClosed() + `rocksdb_optimistictransaction_options_set nname`(txOpts.cPtr, value.ctyp) + +setOpt setSnapshot, bool, uint8 + +proc defaultOptimisticTxOptions*(autoClose = false): OptimisticTxOptionsRef {.inline.} = + let txOpts = createOptimisticTxOptions(autoClose) + + # TODO: set prefered defaults + txOpts + +proc close*(txOpts: OptimisticTxOptionsRef) = + if not txOpts.isClosed(): + rocksdb_optimistictransaction_options_destroy(txOpts.cPtr) + txOpts.cPtr = nil diff --git a/rocksdb/transactions/transaction.nim b/rocksdb/transactions/transaction.nim index 51cc222..41332e3 100644 --- a/rocksdb/transactions/transaction.nim +++ b/rocksdb/transactions/transaction.nim @@ -24,7 +24,7 @@ import ../options/[readopts, writeopts], ../internal/[cftable, utils], ../rocksresult, - ./txopts + ./[txopts, otxopts] export rocksresult @@ -36,6 +36,7 @@ type readOpts: ReadOptionsRef writeOpts: WriteOptionsRef txOpts: TransactionOptionsRef + otxOpts: OptimisticTxOptionsRef defaultCfHandle: ColFamilyHandleRef proc newTransaction*( @@ -43,6 +44,7 @@ proc newTransaction*( readOpts: ReadOptionsRef, writeOpts: WriteOptionsRef, txOpts: TransactionOptionsRef, + otxOpts: OptimisticTxOptionsRef, defaultCfHandle: ColFamilyHandleRef, ): TransactionRef = TransactionRef( @@ -50,6 +52,7 @@ proc newTransaction*( readOpts: readOpts, writeOpts: writeOpts, txOpts: txOpts, + otxOpts: otxOpts, defaultCfHandle: defaultCfHandle, ) @@ -182,3 +185,4 @@ proc close*(tx: TransactionRef) = autoCloseNonNil(tx.readOpts) autoCloseNonNil(tx.writeOpts) autoCloseNonNil(tx.txOpts) + autoCloseNonNil(tx.otxOpts) diff --git a/tests/test_all.nim b/tests/test_all.nim index 62d2fa1..55ce7c1 100644 --- a/tests/test_all.nim +++ b/tests/test_all.nim @@ -18,10 +18,12 @@ import ./options/test_readopts, ./options/test_tableopts, ./options/test_writeopts, + ./transactions/test_otxopts, ./transactions/test_txdbopts, ./transactions/test_txopts, ./test_backup, ./test_columnfamily, + ./test_optimistictxdb, ./test_rocksdb, ./test_rocksiterator, ./test_sstfilewriter, diff --git a/tests/test_helper.nim b/tests/test_helper.nim index 37d2e1e..a7056a7 100644 --- a/tests/test_helper.nim +++ b/tests/test_helper.nim @@ -9,7 +9,7 @@ {.used.} -import std/sequtils, ../rocksdb/backup, ../rocksdb/rocksdb, ../rocksdb/transactiondb +import std/sequtils, ../rocksdb/[backup, rocksdb, transactiondb, optimistictxdb] proc initReadWriteDb*( path: string, columnFamilyNames: openArray[string] = @[] @@ -57,3 +57,17 @@ proc initTransactionDb*( echo res.error() doAssert res.isOk() res.value() + +proc initOptimisticTxDb*( + path: string, columnFamilyNames: openArray[string] = @[] +): OptimisticTxDbRef = + let res = openOptimisticTxDb( + path, + columnFamilies = columnFamilyNames.mapIt( + initColFamilyDescriptor(it, defaultColFamilyOptions(autoClose = true)) + ), + ) + if res.isErr(): + echo res.error() + doAssert res.isOk() + res.value() diff --git a/tests/test_optimistictxdb.nim b/tests/test_optimistictxdb.nim new file mode 100644 index 0000000..7755704 --- /dev/null +++ b/tests/test_optimistictxdb.nim @@ -0,0 +1,291 @@ +# Nim-RocksDB +# Copyright 2024 Status Research & Development GmbH +# Licensed under either of +# +# * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or http://www.apache.org/licenses/LICENSE-2.0) +# * GPL license, version 2.0, ([LICENSE-GPLv2](LICENSE-GPLv2) or https://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html) +# +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +{.used.} + +import std/os, tempfile, unittest2, ../rocksdb/optimistictxdb, ./test_helper + +suite "OptimisticTxDbRef Tests": + const + CF_DEFAULT = "default" + CF_OTHER = "other" + + let + key1 = @[byte(1)] + val1 = @[byte(1)] + key2 = @[byte(2)] + val2 = @[byte(2)] + key3 = @[byte(3)] + val3 = @[byte(3)] + + setup: + let + dbPath = mkdtemp() / "data" + db = initOptimisticTxDb(dbPath, columnFamilyNames = @[CF_OTHER]) + defaultCfHandle = db.getColFamilyHandle(CF_DEFAULT).get() + otherCfHandle = db.getColFamilyHandle(CF_OTHER).get() + + teardown: + db.close() + removeDir($dbPath) + + # test multiple transactions + test "Test rollback using default column family": + var tx = db.beginTransaction() + defer: + tx.close() + check not tx.isClosed() + + check: + tx.put(key1, val1).isOk() + tx.put(key2, val2).isOk() + tx.put(key3, val3).isOk() + + tx.delete(key2).isOk() + not tx.isClosed() + + check: + tx.get(key1).get() == val1 + tx.get(key2).error() == "" + tx.get(key3).get() == val3 + + let res = tx.rollback() + check: + res.isOk() + tx.get(key1).error() == "" + tx.get(key2).error() == "" + tx.get(key3).error() == "" + + test "Test commit using default column family": + var tx = db.beginTransaction() + defer: + tx.close() + check not tx.isClosed() + + check: + tx.put(key1, val1).isOk() + tx.put(key2, val2).isOk() + tx.put(key3, val3).isOk() + + tx.delete(key2).isOk() + not tx.isClosed() + + check: + tx.get(key1).get() == val1 + tx.get(key2).error() == "" + tx.get(key3).get() == val3 + + let res = tx.commit() + check: + res.isOk() + tx.get(key1).get() == val1 + tx.get(key2).error() == "" + tx.get(key3).get() == val3 + + test "Test setting column family in beginTransaction": + var tx = db.beginTransaction(cfHandle = otherCfHandle) + defer: + tx.close() + check not tx.isClosed() + + check: + tx.put(key1, val1).isOk() + tx.put(key2, val2).isOk() + tx.put(key3, val3).isOk() + + tx.delete(key2).isOk() + not tx.isClosed() + + check: + tx.get(key1, defaultCfHandle).error() == "" + tx.get(key2, defaultCfHandle).error() == "" + tx.get(key3, defaultCfHandle).error() == "" + tx.get(key1, otherCfHandle).get() == val1 + tx.get(key2, otherCfHandle).error() == "" + tx.get(key3, otherCfHandle).get() == val3 + + test "Test rollback and commit with multiple transactions": + var tx1 = db.beginTransaction(cfHandle = defaultCfHandle) + defer: + tx1.close() + check not tx1.isClosed() + var tx2 = db.beginTransaction(cfHandle = otherCfHandle) + defer: + tx2.close() + check not tx2.isClosed() + + check: + tx1.put(key1, val1).isOk() + tx1.put(key2, val2).isOk() + tx1.put(key3, val3).isOk() + tx1.delete(key2).isOk() + not tx1.isClosed() + tx2.put(key1, val1).isOk() + tx2.put(key2, val2).isOk() + tx2.put(key3, val3).isOk() + tx2.delete(key2).isOk() + not tx2.isClosed() + + check: + tx1.get(key1, defaultCfHandle).get() == val1 + tx1.get(key2, defaultCfHandle).error() == "" + tx1.get(key3, defaultCfHandle).get() == val3 + tx1.get(key1, otherCfHandle).error() == "" + tx1.get(key2, otherCfHandle).error() == "" + tx1.get(key3, otherCfHandle).error() == "" + + tx2.get(key1, defaultCfHandle).error() == "" + tx2.get(key2, defaultCfHandle).error() == "" + tx2.get(key3, defaultCfHandle).error() == "" + tx2.get(key1, otherCfHandle).get() == val1 + tx2.get(key2, otherCfHandle).error() == "" + tx2.get(key3, otherCfHandle).get() == val3 + + block: + let res = tx1.rollback() + check: + res.isOk() + tx1.get(key1, defaultCfHandle).error() == "" + tx1.get(key2, defaultCfHandle).error() == "" + tx1.get(key3, defaultCfHandle).error() == "" + tx1.get(key1, otherCfHandle).error() == "" + tx1.get(key2, otherCfHandle).error() == "" + tx1.get(key3, otherCfHandle).error() == "" + + block: + let res = tx2.commit() + check: + res.isOk() + tx2.get(key1, defaultCfHandle).error() == "" + tx2.get(key2, defaultCfHandle).error() == "" + tx2.get(key3, defaultCfHandle).error() == "" + tx2.get(key1, otherCfHandle).get() == val1 + tx2.get(key2, otherCfHandle).error() == "" + tx2.get(key3, otherCfHandle).get() == val3 + + test "Test close": + var tx = db.beginTransaction() + + check not tx.isClosed() + tx.close() + check tx.isClosed() + tx.close() + check tx.isClosed() + + check not db.isClosed() + db.close() + check db.isClosed() + db.close() + check db.isClosed() + + test "Test close multiple tx": + var tx1 = db.beginTransaction() + var tx2 = db.beginTransaction() + + check not db.isClosed() + check not tx1.isClosed() + tx1.close() + check tx1.isClosed() + tx1.close() + check tx1.isClosed() + + check not db.isClosed() + check not tx2.isClosed() + tx2.close() + check tx2.isClosed() + tx2.close() + check tx2.isClosed() + + test "Test auto close enabled": + let + dbPath = mkdtemp() / "autoclose-enabled" + dbOpts = defaultDbOptions(autoClose = true) + columnFamilies = + @[ + initColFamilyDescriptor(CF_DEFAULT, defaultColFamilyOptions(autoClose = true)) + ] + db = openOptimisticTxDb(dbPath, dbOpts, columnFamilies).get() + + check: + dbOpts.isClosed() == false + columnFamilies[0].isClosed() == false + db.isClosed() == false + + db.close() + + check: + dbOpts.isClosed() == true + columnFamilies[0].isClosed() == true + db.isClosed() == true + + test "Test auto close enabled": + let + dbPath = mkdtemp() / "autoclose-disabled" + dbOpts = defaultDbOptions(autoClose = false) + columnFamilies = + @[ + initColFamilyDescriptor( + CF_DEFAULT, defaultColFamilyOptions(autoClose = false) + ) + ] + db = openOptimisticTxDb(dbPath, dbOpts, columnFamilies).get() + + check: + dbOpts.isClosed() == false + columnFamilies[0].isClosed() == false + db.isClosed() == false + + db.close() + + check: + dbOpts.isClosed() == false + columnFamilies[0].isClosed() == false + db.isClosed() == true + + test "Test auto close tx enabled": + let + readOpts = defaultReadOptions(autoClose = true) + writeOpts = defaultWriteOptions(autoClose = true) + otxOpts = defaultOptimisticTxOptions(autoClose = true) + tx = db.beginTransaction(readOpts, writeOpts, otxOpts) + + check: + readOpts.isClosed() == false + writeOpts.isClosed() == false + otxOpts.isClosed() == false + tx.isClosed() == false + + tx.close() + + check: + readOpts.isClosed() == true + writeOpts.isClosed() == true + otxOpts.isClosed() == true + tx.isClosed() == true + + test "Test auto close tx disabled": + let + readOpts = defaultReadOptions(autoClose = false) + writeOpts = defaultWriteOptions(autoClose = false) + otxOpts = defaultOptimisticTxOptions(autoClose = false) + tx = db.beginTransaction(readOpts, writeOpts, otxOpts) + + check: + readOpts.isClosed() == false + writeOpts.isClosed() == false + otxOpts.isClosed() == false + tx.isClosed() == false + + tx.close() + + check: + readOpts.isClosed() == false + writeOpts.isClosed() == false + otxOpts.isClosed() == false + tx.isClosed() == true diff --git a/tests/transactions/test_otxopts.nim b/tests/transactions/test_otxopts.nim new file mode 100644 index 0000000..7be0902 --- /dev/null +++ b/tests/transactions/test_otxopts.nim @@ -0,0 +1,36 @@ +# Nim-RocksDB +# Copyright 2024 Status Research & Development GmbH +# Licensed under either of +# +# * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or http://www.apache.org/licenses/LICENSE-2.0) +# * GPL license, version 2.0, ([LICENSE-GPLv2](LICENSE-GPLv2) or https://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html) +# +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +{.used.} + +import unittest2, ../../rocksdb/transactions/otxopts + +suite "OptimisticTxOptionsRef Tests": + test "Test createOptimisticTxOptions": + let txOpts = createOptimisticTxOptions() + + check not txOpts.cPtr.isNil() + + txOpts.close() + + test "Test defaultTransactionOptions": + let txOpts = defaultOptimisticTxOptions() + + check not txOpts.cPtr.isNil() + + txOpts.close() + + test "Test close": + let txOpts = defaultOptimisticTxOptions() + + check not txOpts.isClosed() + txOpts.close() + check txOpts.isClosed() + txOpts.close() + check txOpts.isClosed() diff --git a/tests/transactions/test_txdbopts.nim b/tests/transactions/test_txdbopts.nim index 5619395..b9d56df 100644 --- a/tests/transactions/test_txdbopts.nim +++ b/tests/transactions/test_txdbopts.nim @@ -13,21 +13,21 @@ import unittest2, ../../rocksdb/transactions/txdbopts suite "TransactionDbOptionsRef Tests": test "Test newTransactionDbOptions": - var txDbOpts = createTransactionDbOptions() + let txDbOpts = createTransactionDbOptions() check not txDbOpts.cPtr.isNil() txDbOpts.close() test "Test defaultTransactionDbOptions": - var txDbOpts = defaultTransactionDbOptions() + let txDbOpts = defaultTransactionDbOptions() check not txDbOpts.cPtr.isNil() txDbOpts.close() test "Test close": - var txDbOpts = defaultTransactionDbOptions() + let txDbOpts = defaultTransactionDbOptions() check not txDbOpts.isClosed() txDbOpts.close() diff --git a/tests/transactions/test_txopts.nim b/tests/transactions/test_txopts.nim index 9ea00f7..97cd4de 100644 --- a/tests/transactions/test_txopts.nim +++ b/tests/transactions/test_txopts.nim @@ -13,21 +13,21 @@ import unittest2, ../../rocksdb/transactions/txopts suite "TransactionOptionsRef Tests": test "Test newTransactionOptions": - var txOpts = createTransactionOptions() + let txOpts = createTransactionOptions() check not txOpts.cPtr.isNil() txOpts.close() test "Test defaultTransactionOptions": - var txOpts = defaultTransactionOptions() + let txOpts = defaultTransactionOptions() check not txOpts.cPtr.isNil() txOpts.close() test "Test close": - var txOpts = defaultTransactionOptions() + let txOpts = defaultTransactionOptions() check not txOpts.isClosed() txOpts.close() From 3bcc4ea2b051b130311257ab0b555c529a6ffa8f Mon Sep 17 00:00:00 2001 From: web3-developer <51288821+web3-developer@users.noreply.github.com> Date: Mon, 8 Jul 2024 14:22:53 +0800 Subject: [PATCH 2/5] Add keyMayExist to RocksDbRef. --- rocksdb/rocksdb.nim | 27 ++++++++++++++++++++++++--- tests/test_rocksdb.nim | 23 +++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/rocksdb/rocksdb.nim b/rocksdb/rocksdb.nim index abb60c0..a15c62c 100644 --- a/rocksdb/rocksdb.nim +++ b/rocksdb/rocksdb.nim @@ -305,6 +305,30 @@ proc put*( ok() +proc keyMayExist*( + db: RocksDbRef, key: openArray[byte], cfHandle = db.defaultCfHandle +): RocksDBResult[bool] = + ## If the key definitely does not exist in the database, then this method + ## returns false, otherwise it returns true if the key might exist. That is + ## to say that this method is probabilistic and may return false positives, + ## but never a false negative. This check is potentially lighter-weight than + ## invoking keyExists. + + let keyMayExist = rocksdb_key_may_exist_cf( + db.cPtr, + db.readOpts.cPtr, + cfHandle.cPtr, + cast[cstring](unsafeAddr key[0]), + csize_t(key.len), + nil, + nil, + nil, + 0, + nil, + ).bool + + ok(keyMayExist) + proc keyExists*( db: RocksDbRef, key: openArray[byte], cfHandle = db.defaultCfHandle ): RocksDBResult[bool] = @@ -312,9 +336,6 @@ proc keyExists*( ## Returns a result containing `true` if the key exists or a result ## containing `false` otherwise. - # TODO: Call rocksdb_key_may_exist_cf to improve performance for the case - # when the key does not exist - db.get( key, proc(data: openArray[byte]) = diff --git a/tests/test_rocksdb.nim b/tests/test_rocksdb.nim index d0a8d14..669a29d 100644 --- a/tests/test_rocksdb.nim +++ b/tests/test_rocksdb.nim @@ -324,6 +324,29 @@ suite "RocksDbRef Tests": v.len() == 0 db.get(key5).isErr() + test "Test keyMayExist": + let + key1 = @[byte(1)] # exists with non empty value + val1 = @[byte(1)] + key2 = @[byte(2)] # exists with empty seq value + val2: seq[byte] = @[] + key3 = @[byte(3)] # exists with empty array value + val3: array[0, byte] = [] + key4 = @[byte(4)] # deleted key + key5 = @[byte(5)] # key not created + + check: + db.put(key1, val1).isOk() + db.put(key2, val2).isOk() + db.put(key3, val3).isOk() + db.delete(key4).isOk() + + db.keyMayExist(key1).isOk() + db.keyMayExist(key2).isOk() + db.keyMayExist(key3).isOk() + db.keyMayExist(key4).get() == false + db.keyMayExist(key5).get() == false + test "List column familes": let cfRes1 = listColumnFamilies(dbPath) check: From e3bb8429ad698dd8ce161c0e77b0b90e56411b05 Mon Sep 17 00:00:00 2001 From: web3-developer <51288821+web3-developer@users.noreply.github.com> Date: Mon, 8 Jul 2024 16:48:32 +0800 Subject: [PATCH 3/5] Add support for write batch with index. --- rocksdb.nim | 7 +- rocksdb/rocksdb.nim | 42 ++++++- rocksdb/writebatch.nim | 2 + rocksdb/writebatchwi.nim | 153 +++++++++++++++++++++++++ tests/test_all.nim | 3 +- tests/test_writebatch.nim | 28 +++-- tests/test_writebatchwi.nim | 223 ++++++++++++++++++++++++++++++++++++ 7 files changed, 439 insertions(+), 19 deletions(-) create mode 100644 rocksdb/writebatchwi.nim create mode 100644 tests/test_writebatchwi.nim diff --git a/rocksdb.nim b/rocksdb.nim index 25e3330..92dfe7a 100644 --- a/rocksdb.nim +++ b/rocksdb.nim @@ -9,9 +9,10 @@ import ./rocksdb/[ - backup, columnfamily, rocksdb, rocksiterator, sstfilewriter, transactiondb, - writebatch, + backup, columnfamily, optimistictxdb, rocksdb, rocksiterator, sstfilewriter, + transactiondb, writebatch, writebatchwi, ] export - backup, columnfamily, rocksdb, rocksiterator, sstfilewriter, transactiondb, writebatch + backup, columnfamily, optimistictxdb, rocksdb, rocksiterator, sstfilewriter, + transactiondb, writebatch, writebatchwi diff --git a/rocksdb/rocksdb.nim b/rocksdb/rocksdb.nim index a15c62c..70f120f 100644 --- a/rocksdb/rocksdb.nim +++ b/rocksdb/rocksdb.nim @@ -31,11 +31,11 @@ import ./options/[dbopts, readopts, writeopts], ./columnfamily/[cfopts, cfdescriptor, cfhandle], ./internal/[cftable, utils], - ./rocksiterator, - ./rocksresult, - ./writebatch + ./[rocksiterator, rocksresult, writebatch, writebatchwi] -export rocksresult, dbopts, readopts, writeopts, cfdescriptor, rocksiterator, writebatch +export + rocksresult, dbopts, readopts, writeopts, cfdescriptor, cfhandle, rocksiterator, + writebatch, writebatchwi type RocksDbPtr* = ptr rocksdb_t @@ -371,6 +371,7 @@ proc openIterator*( db: RocksDbRef, cfHandle = db.defaultCfHandle ): RocksDBResult[RocksIteratorRef] = ## Opens an `RocksIteratorRef` for the specified column family. + ## The iterator should be closed using the `close` method after usage. doAssert not db.isClosed() let rocksIterPtr = @@ -382,10 +383,31 @@ proc openWriteBatch*( db: RocksDbReadWriteRef, cfHandle = db.defaultCfHandle ): WriteBatchRef = ## Opens a `WriteBatchRef` which defaults to using the specified column family. + ## The write batch should be closed using the `close` method after usage. doAssert not db.isClosed() createWriteBatch(cfHandle) +proc openWriteBatchWithIndex*( + db: RocksDbReadWriteRef, + reservedBytes = 0, + overwriteKey = false, + cfHandle = db.defaultCfHandle, +): WriteBatchWIRef = + ## Opens a `WriteBatchWIRef` which defaults to using the specified column family. + ## The write batch should be closed using the `close` method after usage. + ## `WriteBatchWIRef` is similar to `WriteBatchRef` but with a binary searchable + ## index built for all the keys inserted which allows reading the data which has + ## been writen to the batch. + ## + ## Optionally set the number of bytes to be reserved for the batch by setting + ## `reservedBytes`. Set `overwriteKey` to true to overwrite the key in the index + ## when inserting a duplicate key, in this way an iterator will never show two + ## entries with the same key. + doAssert not db.isClosed() + + createWriteBatch(reservedBytes, overwriteKey, db.dbOpts, cfHandle) + proc write*(db: RocksDbReadWriteRef, updates: WriteBatchRef): RocksDBResult[void] = ## Apply the updates in the `WriteBatchRef` to the database. doAssert not db.isClosed() @@ -398,6 +420,18 @@ proc write*(db: RocksDbReadWriteRef, updates: WriteBatchRef): RocksDBResult[void ok() +proc write*(db: RocksDbReadWriteRef, updates: WriteBatchWIRef): RocksDBResult[void] = + ## Apply the updates in the `WriteBatchWIRef` to the database. + doAssert not db.isClosed() + + var errors: cstring + rocksdb_write_writebatch_wi( + db.cPtr, db.writeOpts.cPtr, updates.cPtr, cast[cstringArray](errors.addr) + ) + bailOnErrors(errors) + + ok() + proc ingestExternalFile*( db: RocksDbReadWriteRef, filePath: string, cfHandle = db.defaultCfHandle ): RocksDBResult[void] = diff --git a/rocksdb/writebatch.nim b/rocksdb/writebatch.nim index d357792..7126575 100644 --- a/rocksdb/writebatch.nim +++ b/rocksdb/writebatch.nim @@ -8,6 +8,8 @@ # at your option. This file may not be copied, modified, or distributed except according to those terms. ## A `WriteBatchRef` holds a collection of updates to apply atomically to the database. +## It depends on resources from an instance of `RocksDbRef' and therefore should be used +## and closed before the `RocksDbRef` is closed. {.push raises: [].} diff --git a/rocksdb/writebatchwi.nim b/rocksdb/writebatchwi.nim new file mode 100644 index 0000000..fe7fdaf --- /dev/null +++ b/rocksdb/writebatchwi.nim @@ -0,0 +1,153 @@ +# Nim-RocksDB +# Copyright 2024 Status Research & Development GmbH +# Licensed under either of +# +# * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or http://www.apache.org/licenses/LICENSE-2.0) +# * GPL license, version 2.0, ([LICENSE-GPLv2](LICENSE-GPLv2) or https://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html) +# +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +## A `WriteBatchWIRef` holds a collection of updates to apply atomically to the database. +## It depends on resources from an instance of `RocksDbRef' and therefore should be used +## and closed before the `RocksDbRef` is closed. +## +## `WriteBatchWIRef` is similar to `WriteBatchRef` but with a binary searchable index +## built for all the keys inserted which allows reading the data which has been writen +## to the batch. + +{.push raises: [].} + +import ./lib/librocksdb, ./internal/[cftable, utils], ./options/dbopts, ./rocksresult + +export rocksresult + +type + WriteBatchWIPtr* = ptr rocksdb_writebatch_wi_t + + WriteBatchWIRef* = ref object + cPtr: WriteBatchWIPtr + dbOpts: DbOptionsRef + defaultCfHandle: ColFamilyHandleRef + +proc createWriteBatch*( + reservedBytes: int, + overwriteKey: bool, + dbOpts: DbOptionsRef, + defaultCfHandle: ColFamilyHandleRef, +): WriteBatchWIRef = + WriteBatchWIRef( + cPtr: rocksdb_writebatch_wi_create(reservedBytes.csize_t, overwriteKey.uint8), + dbOpts: dbOpts, + defaultCfHandle: defaultCfHandle, + ) + +proc isClosed*(batch: WriteBatchWIRef): bool {.inline.} = + ## Returns `true` if the `WriteBatchWIRef` has been closed and `false` otherwise. + batch.cPtr.isNil() + +proc cPtr*(batch: WriteBatchWIRef): WriteBatchWIPtr = + ## Get the underlying database pointer. + doAssert not batch.isClosed() + batch.cPtr + +proc clear*(batch: WriteBatchWIRef) = + ## Clears the write batch. + doAssert not batch.isClosed() + rocksdb_writebatch_wi_clear(batch.cPtr) + +proc count*(batch: WriteBatchWIRef): int = + ## Get the number of updates in the write batch. + doAssert not batch.isClosed() + rocksdb_writebatch_wi_count(batch.cPtr).int + +proc put*( + batch: WriteBatchWIRef, key, val: openArray[byte], cfHandle = batch.defaultCfHandle +): RocksDBResult[void] = + ## Add a put operation to the write batch. + + if key.len() == 0: + return err("rocksdb: key is empty") + + rocksdb_writebatch_wi_put_cf( + batch.cPtr, + cfHandle.cPtr, + cast[cstring](unsafeAddr key[0]), + csize_t(key.len), + cast[cstring](if val.len > 0: + unsafeAddr val[0] + else: + nil + ), + csize_t(val.len), + ) + + ok() + +proc delete*( + batch: WriteBatchWIRef, key: openArray[byte], cfHandle = batch.defaultCfHandle +): RocksDBResult[void] = + ## Add a delete operation to the write batch. + + if key.len() == 0: + return err("rocksdb: key is empty") + + rocksdb_writebatch_wi_delete_cf( + batch.cPtr, cfHandle.cPtr, cast[cstring](unsafeAddr key[0]), csize_t(key.len) + ) + + ok() + +proc get*( + batch: WriteBatchWIRef, + key: openArray[byte], + onData: DataProc, + cfHandle = batch.defaultCfHandle, +): RocksDBResult[bool] = + ## Get the value for a given key from the batch using the provided + ## `onData` callback. + + if key.len() == 0: + return err("rocksdb: key is empty") + + var + len: csize_t + errors: cstring + let data = rocksdb_writebatch_wi_get_from_batch_cf( + batch.cPtr, + batch.dbOpts.cPtr, + cfHandle.cPtr, + cast[cstring](unsafeAddr key[0]), + csize_t(key.len), + len.addr, + cast[cstringArray](errors.addr), + ) + bailOnErrors(errors) + + if data.isNil(): + doAssert len == 0 + ok(false) + else: + onData(toOpenArrayByte(data, 0, len.int - 1)) + rocksdb_free(data) + ok(true) + +proc get*( + batch: WriteBatchWIRef, key: openArray[byte], cfHandle = batch.defaultCfHandle +): RocksDBResult[seq[byte]] = + ## Get the value for a given key from the batch. + + var dataRes: RocksDBResult[seq[byte]] + proc onData(data: openArray[byte]) = + dataRes.ok(@data) + + let res = batch.get(key, onData, cfHandle) + if res.isOk(): + return dataRes + + dataRes.err(res.error()) + +proc close*(batch: WriteBatchWIRef) = + ## Close the `WriteBatchWIRef`. + if not batch.isClosed(): + rocksdb_writebatch_wi_destroy(batch.cPtr) + batch.cPtr = nil diff --git a/tests/test_all.nim b/tests/test_all.nim index 55ce7c1..e94f10a 100644 --- a/tests/test_all.nim +++ b/tests/test_all.nim @@ -28,4 +28,5 @@ import ./test_rocksiterator, ./test_sstfilewriter, ./test_transactiondb, - ./test_writebatch + ./test_writebatch, + ./test_writebatchwi diff --git a/tests/test_writebatch.nim b/tests/test_writebatch.nim index ac219e5..d0150bb 100644 --- a/tests/test_writebatch.nim +++ b/tests/test_writebatch.nim @@ -36,7 +36,7 @@ suite "WriteBatchRef Tests": removeDir($dbPath) test "Test writing batch to the default column family": - var batch = db.openWriteBatch() + let batch = db.openWriteBatch() defer: batch.close() check not batch.isClosed() @@ -65,7 +65,7 @@ suite "WriteBatchRef Tests": not batch.isClosed() test "Test writing batch to column family": - var batch = db.openWriteBatch() + let batch = db.openWriteBatch() defer: batch.close() check not batch.isClosed() @@ -93,7 +93,7 @@ suite "WriteBatchRef Tests": not batch.isClosed() test "Test writing to multiple column families in single batch": - var batch = db.openWriteBatch() + let batch = db.openWriteBatch() defer: batch.close() check not batch.isClosed() @@ -123,17 +123,16 @@ suite "WriteBatchRef Tests": not batch.isClosed() test "Test writing to multiple column families in multiple batches": - var batch1 = db.openWriteBatch() + let + batch1 = db.openWriteBatch() + batch2 = db.openWriteBatch() defer: batch1.close() - check not batch1.isClosed() - - var batch2 = db.openWriteBatch() - defer: batch2.close() - check not batch2.isClosed() check: + not batch1.isClosed() + not batch2.isClosed() batch1.put(key1, val1).isOk() batch1.delete(key2, otherCfHandle).isOk() batch1.put(key3, val3, otherCfHandle).isOk() @@ -155,8 +154,14 @@ suite "WriteBatchRef Tests": db.keyExists(key2, otherCfHandle).get() == false db.get(key3, otherCfHandle).get() == val3 + # Write batch is unchanged after write + batch1.count() == 3 + batch2.count() == 3 + not batch1.isClosed() + not batch2.isClosed() + test "Test write empty batch": - var batch = db.openWriteBatch() + let batch = db.openWriteBatch() defer: batch.close() check not batch.isClosed() @@ -166,9 +171,10 @@ suite "WriteBatchRef Tests": check: res1.isOk() batch.count() == 0 + not batch.isClosed() test "Test close": - var batch = db.openWriteBatch() + let batch = db.openWriteBatch() check not batch.isClosed() batch.close() diff --git a/tests/test_writebatchwi.nim b/tests/test_writebatchwi.nim new file mode 100644 index 0000000..09afd52 --- /dev/null +++ b/tests/test_writebatchwi.nim @@ -0,0 +1,223 @@ +# Nim-RocksDB +# Copyright 2024 Status Research & Development GmbH +# Licensed under either of +# +# * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or http://www.apache.org/licenses/LICENSE-2.0) +# * GPL license, version 2.0, ([LICENSE-GPLv2](LICENSE-GPLv2) or https://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html) +# +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +{.used.} + +import std/os, tempfile, unittest2, ../rocksdb/[rocksdb, writebatchwi], ./test_helper + +suite "WriteBatchWIRef Tests": + const + CF_DEFAULT = "default" + CF_OTHER = "other" + + let + key1 = @[byte(1)] + val1 = @[byte(1)] + key2 = @[byte(2)] + val2 = @[byte(2)] + key3 = @[byte(3)] + val3 = @[byte(3)] + + setup: + let + dbPath = mkdtemp() / "data" + db = initReadWriteDb(dbPath, columnFamilyNames = @[CF_DEFAULT, CF_OTHER]) + defaultCfHandle = db.getColFamilyHandle(CF_DEFAULT).get() + otherCfHandle = db.getColFamilyHandle(CF_OTHER).get() + + teardown: + db.close() + removeDir($dbPath) + + test "Test writing batch to the default column family": + let batch = db.openWriteBatchWithIndex() + defer: + batch.close() + check not batch.isClosed() + + check: + batch.put(key1, val1).isOk() + batch.put(key2, val2).isOk() + batch.put(key3, val3).isOk() + batch.count() == 3 + + batch.delete(key2).isOk() + batch.count() == 4 + not batch.isClosed() + + batch.get(key1).get() == val1 + batch.get(key2).isErr() + batch.get(key3).get() == val3 + + let res = db.write(batch) + check: + res.isOk() + db.write(batch).isOk() # test that it's idempotent + db.get(key1).get() == val1 + db.keyExists(key2).get() == false + db.get(key3).get() == val3 + + batch.get(key1).get() == val1 + batch.get(key2).isErr() + batch.get(key3).get() == val3 + + batch.clear() + check: + batch.count() == 0 + not batch.isClosed() + + test "Test writing batch to column family": + let batch = db.openWriteBatchWithIndex() + defer: + batch.close() + check not batch.isClosed() + + check: + batch.put(key1, val1, otherCfHandle).isOk() + batch.put(key2, val2, otherCfHandle).isOk() + batch.put(key3, val3, otherCfHandle).isOk() + batch.count() == 3 + + batch.delete(key2, otherCfHandle).isOk() + batch.count() == 4 + not batch.isClosed() + + batch.get(key1, otherCfHandle).get() == val1 + batch.get(key2, otherCfHandle).isErr() + batch.get(key3, otherCfHandle).get() == val3 + + let res = db.write(batch) + check: + res.isOk() + db.get(key1, otherCfHandle).get() == val1 + db.keyExists(key2, otherCfHandle).get() == false + db.get(key3, otherCfHandle).get() == val3 + + batch.get(key1, otherCfHandle).get() == val1 + batch.get(key2, otherCfHandle).isErr() + batch.get(key3, otherCfHandle).get() == val3 + + batch.clear() + check: + batch.count() == 0 + not batch.isClosed() + + test "Test writing to multiple column families in single batch": + let batch = db.openWriteBatchWithIndex() + defer: + batch.close() + check not batch.isClosed() + + check: + batch.put(key1, val1, defaultCfHandle).isOk() + batch.put(key1, val1, otherCfHandle).isOk() + batch.put(key2, val2, otherCfHandle).isOk() + batch.put(key3, val3, otherCfHandle).isOk() + batch.count() == 4 + + batch.delete(key2, otherCfHandle).isOk() + batch.count() == 5 + not batch.isClosed() + + let res = db.write(batch) + check: + res.isOk() + db.get(key1, defaultCfHandle).get() == val1 + db.get(key1, otherCfHandle).get() == val1 + db.keyExists(key2, otherCfHandle).get() == false + db.get(key3, otherCfHandle).get() == val3 + + batch.clear() + check: + batch.count() == 0 + not batch.isClosed() + + test "Test writing to multiple column families in multiple batches": + let + batch1 = db.openWriteBatchWithIndex() + batch2 = db.openWriteBatchWithIndex() + defer: + batch1.close() + batch2.close() + + check: + not batch1.isClosed() + not batch2.isClosed() + batch1.put(key1, val1).isOk() + batch1.delete(key2, otherCfHandle).isOk() + batch1.put(key3, val3, otherCfHandle).isOk() + batch2.put(key1, val1, otherCfHandle).isOk() + batch2.delete(key1, otherCfHandle).isOk() + batch2.put(key3, val3).isOk() + batch1.count() == 3 + batch2.count() == 3 + + let res1 = db.write(batch1) + let res2 = db.write(batch2) + check: + res1.isOk() + res2.isOk() + db.get(key1).get() == val1 + db.keyExists(key2).get() == false + db.get(key3).get() == val3 + db.keyExists(key1, otherCfHandle).get() == false + db.keyExists(key2, otherCfHandle).get() == false + db.get(key3, otherCfHandle).get() == val3 + + # Write batch is unchanged after write + batch1.count() == 3 + batch2.count() == 3 + not batch1.isClosed() + not batch2.isClosed() + + test "Test write empty batch": + let batch = db.openWriteBatchWithIndex() + defer: + batch.close() + check not batch.isClosed() + + check batch.count() == 0 + let res1 = db.write(batch) + check: + res1.isOk() + batch.count() == 0 + not batch.isClosed() + + test "Test multiple writes to same key": + let + batch1 = db.openWriteBatchWithIndex(overwriteKey = false) + batch2 = db.openWriteBatchWithIndex(overwriteKey = true) + defer: + batch1.close() + batch2.close() + check: + not batch1.isClosed() + not batch2.isClosed() + + check: + batch1.put(key1, val1).isOk() + batch1.delete(key1).isOk() + batch1.put(key1, val3).isOk() + batch1.count() == 3 + batch1.get(key1).get() == val3 + + batch2.put(key1, val3).isOk() + batch2.put(key1, val2).isOk() + batch2.put(key1, val1).isOk() + batch2.count() == 3 + batch2.get(key1).get() == val1 + + test "Test close": + let batch = db.openWriteBatchWithIndex() + + check not batch.isClosed() + batch.close() + check batch.isClosed() + batch.close() + check batch.isClosed() From a854cd08d49c26f9c46fe0c6c78f90e45b263454 Mon Sep 17 00:00:00 2001 From: web3-developer <51288821+web3-developer@users.noreply.github.com> Date: Mon, 8 Jul 2024 16:54:47 +0800 Subject: [PATCH 4/5] Allow empty keys to be used in API. --- rocksdb/rocksdb.nim | 9 --------- rocksdb/transactions/transaction.nim | 9 --------- rocksdb/writebatch.nim | 6 ------ rocksdb/writebatchwi.nim | 9 --------- 4 files changed, 33 deletions(-) diff --git a/rocksdb/rocksdb.nim b/rocksdb/rocksdb.nim index 70f120f..638db93 100644 --- a/rocksdb/rocksdb.nim +++ b/rocksdb/rocksdb.nim @@ -236,9 +236,6 @@ proc get*( ## The `onData` callback reduces the number of copies and therefore should be ## preferred if performance is required. - if key.len() == 0: - return err("rocksdb: key is empty") - var len: csize_t errors: cstring @@ -283,9 +280,6 @@ proc put*( ): RocksDBResult[void] = ## Put the value for the given key into the specified column family. - if key.len() == 0: - return err("rocksdb: key is empty") - var errors: cstring rocksdb_put_cf( db.cPtr, @@ -351,9 +345,6 @@ proc delete*( ## If the value does not exist, the delete will be a no-op. ## To check if the value exists before or after a delete, use `keyExists`. - if key.len() == 0: - return err("rocksdb: key is empty") - var errors: cstring rocksdb_delete_cf( db.cPtr, diff --git a/rocksdb/transactions/transaction.nim b/rocksdb/transactions/transaction.nim index 41332e3..65cc4c9 100644 --- a/rocksdb/transactions/transaction.nim +++ b/rocksdb/transactions/transaction.nim @@ -69,9 +69,6 @@ proc get*( ## Get the value for a given key from the transaction using the provided ## `onData` callback. - if key.len() == 0: - return err("rocksdb: key is empty") - var len: csize_t errors: cstring @@ -114,9 +111,6 @@ proc put*( ): RocksDBResult[void] = ## Put the value for the given key into the transaction. - if key.len() == 0: - return err("rocksdb: key is empty") - var errors: cstring rocksdb_transaction_put_cf( tx.cPtr, @@ -140,9 +134,6 @@ proc delete*( ): RocksDBResult[void] = ## Delete the value for the given key from the transaction. - if key.len() == 0: - return err("rocksdb: key is empty") - var errors: cstring rocksdb_transaction_delete_cf( tx.cPtr, diff --git a/rocksdb/writebatch.nim b/rocksdb/writebatch.nim index 7126575..c065dbd 100644 --- a/rocksdb/writebatch.nim +++ b/rocksdb/writebatch.nim @@ -51,9 +51,6 @@ proc put*( ): RocksDBResult[void] = ## Add a put operation to the write batch. - if key.len() == 0: - return err("rocksdb: key is empty") - rocksdb_writebatch_put_cf( batch.cPtr, cfHandle.cPtr, @@ -74,9 +71,6 @@ proc delete*( ): RocksDBResult[void] = ## Add a delete operation to the write batch. - if key.len() == 0: - return err("rocksdb: key is empty") - rocksdb_writebatch_delete_cf( batch.cPtr, cfHandle.cPtr, cast[cstring](unsafeAddr key[0]), csize_t(key.len) ) diff --git a/rocksdb/writebatchwi.nim b/rocksdb/writebatchwi.nim index fe7fdaf..e251f0f 100644 --- a/rocksdb/writebatchwi.nim +++ b/rocksdb/writebatchwi.nim @@ -65,9 +65,6 @@ proc put*( ): RocksDBResult[void] = ## Add a put operation to the write batch. - if key.len() == 0: - return err("rocksdb: key is empty") - rocksdb_writebatch_wi_put_cf( batch.cPtr, cfHandle.cPtr, @@ -88,9 +85,6 @@ proc delete*( ): RocksDBResult[void] = ## Add a delete operation to the write batch. - if key.len() == 0: - return err("rocksdb: key is empty") - rocksdb_writebatch_wi_delete_cf( batch.cPtr, cfHandle.cPtr, cast[cstring](unsafeAddr key[0]), csize_t(key.len) ) @@ -106,9 +100,6 @@ proc get*( ## Get the value for a given key from the batch using the provided ## `onData` callback. - if key.len() == 0: - return err("rocksdb: key is empty") - var len: csize_t errors: cstring From e99428d3f3f85be245a8378226a9e0d5088b2170 Mon Sep 17 00:00:00 2001 From: web3-developer <51288821+web3-developer@users.noreply.github.com> Date: Mon, 8 Jul 2024 21:34:35 +0800 Subject: [PATCH 5/5] Complete support for empty keys and add tests. --- rocksdb/internal/utils.nim | 6 ++++++ rocksdb/rocksdb.nim | 14 +++++--------- rocksdb/rocksiterator.nim | 2 +- rocksdb/sstfilewriter.nim | 6 +++--- rocksdb/transactions/transaction.nim | 12 ++++-------- rocksdb/writebatch.nim | 12 ++++-------- rocksdb/writebatchwi.nim | 12 ++++-------- tests/test_rocksdb.nim | 9 +++++++++ tests/test_rocksiterator.nim | 14 ++++++++++++++ tests/test_sstfilewriter.nim | 13 +++++++++++++ tests/test_transactiondb.nim | 14 +++++++++++++- tests/test_writebatch.nim | 14 ++++++++++++++ tests/test_writebatchwi.nim | 12 ++++++++++++ 13 files changed, 102 insertions(+), 38 deletions(-) diff --git a/rocksdb/internal/utils.nim b/rocksdb/internal/utils.nim index e4ba78a..a9581ab 100644 --- a/rocksdb/internal/utils.nim +++ b/rocksdb/internal/utils.nim @@ -38,3 +38,9 @@ template bailOnErrors*(errors: cstring): auto = let res = err($(errors)) rocksdb_free(errors) return res + +template unsafeAddrOrNil*(s: openArray[byte]): auto = + if s.len > 0: + unsafeAddr s[0] + else: + nil diff --git a/rocksdb/rocksdb.nim b/rocksdb/rocksdb.nim index 638db93..d6117a5 100644 --- a/rocksdb/rocksdb.nim +++ b/rocksdb/rocksdb.nim @@ -243,7 +243,7 @@ proc get*( db.cPtr, db.readOpts.cPtr, cfHandle.cPtr, - cast[cstring](unsafeAddr key[0]), + cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len), len.addr, cast[cstringArray](errors.addr), @@ -285,13 +285,9 @@ proc put*( db.cPtr, db.writeOpts.cPtr, cfHandle.cPtr, - cast[cstring](unsafeAddr key[0]), + cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len), - cast[cstring](if val.len > 0: - unsafeAddr val[0] - else: - nil - ), + cast[cstring](val.unsafeAddrOrNil()), csize_t(val.len), cast[cstringArray](errors.addr), ) @@ -312,7 +308,7 @@ proc keyMayExist*( db.cPtr, db.readOpts.cPtr, cfHandle.cPtr, - cast[cstring](unsafeAddr key[0]), + cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len), nil, nil, @@ -350,7 +346,7 @@ proc delete*( db.cPtr, db.writeOpts.cPtr, cfHandle.cPtr, - cast[cstring](unsafeAddr key[0]), + cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len), cast[cstringArray](errors.addr), ) diff --git a/rocksdb/rocksiterator.nim b/rocksdb/rocksiterator.nim index 9f0371b..2a2129a 100644 --- a/rocksdb/rocksiterator.nim +++ b/rocksdb/rocksiterator.nim @@ -46,7 +46,7 @@ proc seekToKey*(iter: RocksIteratorRef, key: openArray[byte]) = ## invalid. ## doAssert not iter.isClosed() - rocksdb_iter_seek(iter.cPtr, cast[cstring](unsafeAddr key[0]), csize_t(key.len)) + rocksdb_iter_seek(iter.cPtr, cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len)) proc seekToFirst*(iter: RocksIteratorRef) = ## Seeks to the first entry in the column family. diff --git a/rocksdb/sstfilewriter.nim b/rocksdb/sstfilewriter.nim index 107cd48..7d6c6c5 100644 --- a/rocksdb/sstfilewriter.nim +++ b/rocksdb/sstfilewriter.nim @@ -61,9 +61,9 @@ proc put*( var errors: cstring rocksdb_sstfilewriter_put( writer.cPtr, - cast[cstring](unsafeAddr key[0]), + cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len), - cast[cstring](unsafeAddr val[0]), + cast[cstring](val.unsafeAddrOrNil()), csize_t(val.len), cast[cstringArray](errors.addr), ) @@ -77,7 +77,7 @@ proc delete*(writer: SstFileWriterRef, key: openArray[byte]): RocksDBResult[void var errors: cstring rocksdb_sstfilewriter_delete( writer.cPtr, - cast[cstring](unsafeAddr key[0]), + cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len), cast[cstringArray](errors.addr), ) diff --git a/rocksdb/transactions/transaction.nim b/rocksdb/transactions/transaction.nim index 65cc4c9..e0db707 100644 --- a/rocksdb/transactions/transaction.nim +++ b/rocksdb/transactions/transaction.nim @@ -76,7 +76,7 @@ proc get*( tx.cPtr, tx.readOpts.cPtr, cfHandle.cPtr, - cast[cstring](unsafeAddr key[0]), + cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len), len.addr, cast[cstringArray](errors.addr), @@ -115,13 +115,9 @@ proc put*( rocksdb_transaction_put_cf( tx.cPtr, cfHandle.cPtr, - cast[cstring](unsafeAddr key[0]), + cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len), - cast[cstring](if val.len > 0: - unsafeAddr val[0] - else: - nil - ), + cast[cstring](val.unsafeAddrOrNil()), csize_t(val.len), cast[cstringArray](errors.addr), ) @@ -138,7 +134,7 @@ proc delete*( rocksdb_transaction_delete_cf( tx.cPtr, cfHandle.cPtr, - cast[cstring](unsafeAddr key[0]), + cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len), cast[cstringArray](errors.addr), ) diff --git a/rocksdb/writebatch.nim b/rocksdb/writebatch.nim index c065dbd..c302ebd 100644 --- a/rocksdb/writebatch.nim +++ b/rocksdb/writebatch.nim @@ -13,7 +13,7 @@ {.push raises: [].} -import ./lib/librocksdb, ./internal/cftable, ./rocksresult +import ./lib/librocksdb, ./internal/[cftable, utils], ./rocksresult export rocksresult @@ -54,13 +54,9 @@ proc put*( rocksdb_writebatch_put_cf( batch.cPtr, cfHandle.cPtr, - cast[cstring](unsafeAddr key[0]), + cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len), - cast[cstring](if val.len > 0: - unsafeAddr val[0] - else: - nil - ), + cast[cstring](val.unsafeAddrOrNil()), csize_t(val.len), ) @@ -72,7 +68,7 @@ proc delete*( ## Add a delete operation to the write batch. rocksdb_writebatch_delete_cf( - batch.cPtr, cfHandle.cPtr, cast[cstring](unsafeAddr key[0]), csize_t(key.len) + batch.cPtr, cfHandle.cPtr, cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len) ) ok() diff --git a/rocksdb/writebatchwi.nim b/rocksdb/writebatchwi.nim index e251f0f..e4ec721 100644 --- a/rocksdb/writebatchwi.nim +++ b/rocksdb/writebatchwi.nim @@ -68,13 +68,9 @@ proc put*( rocksdb_writebatch_wi_put_cf( batch.cPtr, cfHandle.cPtr, - cast[cstring](unsafeAddr key[0]), + cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len), - cast[cstring](if val.len > 0: - unsafeAddr val[0] - else: - nil - ), + cast[cstring](val.unsafeAddrOrNil()), csize_t(val.len), ) @@ -86,7 +82,7 @@ proc delete*( ## Add a delete operation to the write batch. rocksdb_writebatch_wi_delete_cf( - batch.cPtr, cfHandle.cPtr, cast[cstring](unsafeAddr key[0]), csize_t(key.len) + batch.cPtr, cfHandle.cPtr, cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len) ) ok() @@ -107,7 +103,7 @@ proc get*( batch.cPtr, batch.dbOpts.cPtr, cfHandle.cPtr, - cast[cstring](unsafeAddr key[0]), + cast[cstring](key.unsafeAddrOrNil()), csize_t(key.len), len.addr, cast[cstringArray](errors.addr), diff --git a/tests/test_rocksdb.nim b/tests/test_rocksdb.nim index 669a29d..9b6094f 100644 --- a/tests/test_rocksdb.nim +++ b/tests/test_rocksdb.nim @@ -347,6 +347,15 @@ suite "RocksDbRef Tests": db.keyMayExist(key4).get() == false db.keyMayExist(key5).get() == false + test "Put, get and delete empty key": + let empty: seq[byte] = @[] + + check: + db.put(empty, val).isOk() + db.get(empty).get() == val + db.delete(empty).isOk() + db.get(empty).isErr() + test "List column familes": let cfRes1 = listColumnFamilies(dbPath) check: diff --git a/tests/test_rocksiterator.nim b/tests/test_rocksiterator.nim index b8c2fae..cfacf6c 100644 --- a/tests/test_rocksiterator.nim +++ b/tests/test_rocksiterator.nim @@ -165,6 +165,20 @@ suite "RocksIteratorRef Tests": iter.key() == key2 iter.value() == val2 + test "Seek to empty key": + let empty: seq[byte] = @[] + check db.put(empty, val1).isOk() + + let iter = db.openIterator().get() + defer: + iter.close() + + iter.seekToKey(empty) + check: + iter.isValid() + iter.key() == empty + iter.value() == val1 + test "Empty column family": let res = db.openIterator(emptyCfHandle) check res.isOk() diff --git a/tests/test_sstfilewriter.nim b/tests/test_sstfilewriter.nim index 187affe..3cd33a0 100644 --- a/tests/test_sstfilewriter.nim +++ b/tests/test_sstfilewriter.nim @@ -76,6 +76,19 @@ suite "SstFileWriterRef Tests": db.get(key2, otherCfHandle).get() == val2 db.get(key3, otherCfHandle).get() == val3 + test "Put, get and delete empty key": + let writer = openSstFileWriter(sstFilePath).get() + defer: + writer.close() + + let empty: seq[byte] = @[] + check: + writer.put(empty, val1).isOk() + writer.finish().isOk() + db.ingestExternalFile(sstFilePath).isOk() + db.keyExists(empty).get() == true + db.get(empty).get() == val1 + test "Test close": let res = openSstFileWriter(sstFilePath) check res.isOk() diff --git a/tests/test_transactiondb.nim b/tests/test_transactiondb.nim index 2ae0132..76d3d14 100644 --- a/tests/test_transactiondb.nim +++ b/tests/test_transactiondb.nim @@ -169,6 +169,18 @@ suite "TransactionDbRef Tests": tx2.get(key2, otherCfHandle).error() == "" tx2.get(key3, otherCfHandle).get() == val3 + test "Put, get and delete empty key": + let tx = db.beginTransaction() + defer: + tx.close() + + let empty: seq[byte] = @[] + check: + tx.put(empty, val1).isOk() + tx.get(empty).get() == val1 + tx.delete(empty).isOk() + tx.get(empty).isErr() + test "Test close": var tx = db.beginTransaction() @@ -227,7 +239,7 @@ suite "TransactionDbRef Tests": columnFamilies[0].isClosed() == true db.isClosed() == true - test "Test auto close enabled": + test "Test auto close disabled": let dbPath = mkdtemp() / "autoclose-disabled" dbOpts = defaultDbOptions(autoClose = false) diff --git a/tests/test_writebatch.nim b/tests/test_writebatch.nim index d0150bb..fc55962 100644 --- a/tests/test_writebatch.nim +++ b/tests/test_writebatch.nim @@ -160,6 +160,20 @@ suite "WriteBatchRef Tests": not batch1.isClosed() not batch2.isClosed() + test "Put, get and delete empty key": + let batch = db.openWriteBatch() + defer: + batch.close() + + let empty: seq[byte] = @[] + check: + batch.put(empty, val1).isOk() + db.write(batch).isOk() + db.get(empty).get() == val1 + batch.delete(empty).isOk() + db.write(batch).isOk() + db.get(empty).isErr() + test "Test write empty batch": let batch = db.openWriteBatch() defer: diff --git a/tests/test_writebatchwi.nim b/tests/test_writebatchwi.nim index 09afd52..457d06d 100644 --- a/tests/test_writebatchwi.nim +++ b/tests/test_writebatchwi.nim @@ -213,6 +213,18 @@ suite "WriteBatchWIRef Tests": batch2.count() == 3 batch2.get(key1).get() == val1 + test "Put, get and delete empty key": + let batch = db.openWriteBatchWithIndex() + defer: + batch.close() + + let empty: seq[byte] = @[] + check: + batch.put(empty, val1).isOk() + batch.get(empty).get() == val1 + batch.delete(empty).isOk() + batch.get(empty).isErr() + test "Test close": let batch = db.openWriteBatchWithIndex()