Skip to content

Commit

Permalink
Revert some changes from prior PR #48 which enable memory leak of opt…
Browse files Browse the repository at this point in the history
…ions when not cleaned up manually.
  • Loading branch information
bhartnett committed Jun 27, 2024
1 parent 9f60859 commit 953d9e8
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 43 deletions.
27 changes: 7 additions & 20 deletions rocksdb/rocksdb.nim
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ proc listColumnFamilies*(

proc openRocksDb*(
path: string,
dbOpts = DbOptionsRef(nil),
readOpts = ReadOptionsRef(nil),
writeOpts = WriteOptionsRef(nil),
dbOpts = defaultDbOptions(),
readOpts = defaultReadOptions(),
writeOpts = defaultWriteOptions(),
columnFamilies: openArray[ColFamilyDescriptor] = [],
): RocksDBResult[RocksDbReadWriteRef] =
## Open a RocksDB instance in read-write mode. If `columnFamilies` is empty
Expand All @@ -114,10 +114,6 @@ proc openRocksDb*(
## By default, column families will be created if they don't yet exist.
## All existing column families must be specified if the database has
## previously created any column families.
let useDbOpts = (if dbOpts.isNil: defaultDbOptions() else: dbOpts)
defer:
if dbOpts.isNil:
useDbOpts.close()

var cfs = columnFamilies.toSeq()
if DEFAULT_COLUMN_FAMILY_NAME notin columnFamilies.mapIt(it.name()):
Expand All @@ -129,7 +125,7 @@ proc openRocksDb*(
cfHandles = newSeq[ColFamilyHandlePtr](cfs.len)
errors: cstring
let rocksDbPtr = rocksdb_open_column_families(
useDbOpts.cPtr,
dbOpts.cPtr,
path.cstring,
cfNames.len().cint,
cast[cstringArray](cfNames[0].addr),
Expand All @@ -140,9 +136,6 @@ proc openRocksDb*(
bailOnErrors(errors)

let
dbOpts = useDbOpts # don't close on exit
readOpts = (if readOpts.isNil: defaultReadOptions() else: readOpts)
writeOpts = (if writeOpts.isNil: defaultWriteOptions() else: writeOpts)
cfTable = newColFamilyTable(cfNames.mapIt($it), cfHandles)
db = RocksDbReadWriteRef(
lock: createLock(),
Expand All @@ -159,8 +152,8 @@ proc openRocksDb*(

proc openRocksDbReadOnly*(
path: string,
dbOpts = DbOptionsRef(nil),
readOpts = ReadOptionsRef(nil),
dbOpts = defaultDbOptions(),
readOpts = defaultReadOptions(),
columnFamilies: openArray[ColFamilyDescriptor] = [],
errorIfWalFileExists = false,
): RocksDBResult[RocksDbReadOnlyRef] =
Expand All @@ -170,10 +163,6 @@ proc openRocksDbReadOnly*(
## families will be created if they don't yet exist. If the database already
## contains any column families, then all or a subset of the existing column
## families can be opened for reading.
let useDbOpts = (if dbOpts.isNil: defaultDbOptions() else: dbOpts)
defer:
if dbOpts.isNil:
useDbOpts.close()

var cfs = columnFamilies.toSeq()
if DEFAULT_COLUMN_FAMILY_NAME notin columnFamilies.mapIt(it.name()):
Expand All @@ -185,7 +174,7 @@ proc openRocksDbReadOnly*(
cfHandles = newSeq[ColFamilyHandlePtr](cfs.len)
errors: cstring
let rocksDbPtr = rocksdb_open_for_read_only_column_families(
useDbOpts.cPtr,
dbOpts.cPtr,
path.cstring,
cfNames.len().cint,
cast[cstringArray](cfNames[0].addr),
Expand All @@ -197,8 +186,6 @@ proc openRocksDbReadOnly*(
bailOnErrors(errors)

let
dbOpts = useDbOpts # don't close on exit
readOpts = (if readOpts.isNil: defaultReadOptions() else: readOpts)
cfTable = newColFamilyTable(cfNames.mapIt($it), cfHandles)
db = RocksDbReadOnlyRef(
lock: createLock(),
Expand Down
4 changes: 2 additions & 2 deletions rocksdb/sstfilewriter.nim
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,9 @@ type
dbOpts: DbOptionsRef

proc openSstFileWriter*(
filePath: string, dbOpts = DbOptionsRef(nil)
filePath: string, dbOpts = defaultDbOptions()
): RocksDBResult[SstFileWriterRef] =
## Creates a new `SstFileWriterRef` and opens the file at the given `filePath`.
let dbOpts = (if dbOpts.isNil: defaultDbOptions() else: dbOpts)
doAssert not dbOpts.isClosed()

let envOptsPtr = rocksdb_envoptions_create()
Expand Down Expand Up @@ -95,6 +94,7 @@ proc finish*(writer: SstFileWriterRef): RocksDBResult[void] =
proc close*(writer: SstFileWriterRef) =
## Closes the `SstFileWriterRef`.
if not writer.isClosed():
writer.dbOpts.close()
rocksdb_envoptions_destroy(writer.envOptsPtr)
writer.envOptsPtr = nil
rocksdb_sstfilewriter_destroy(writer.cPtr)
Expand Down
25 changes: 5 additions & 20 deletions rocksdb/transactiondb.nim
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,13 @@ type

proc openTransactionDb*(
path: string,
dbOpts = DbOptionsRef(nil),
txDbOpts = TransactionDbOptionsRef(nil),
dbOpts = defaultDbOptions(),
txDbOpts = defaultTransactionDbOptions(),
columnFamilies: openArray[ColFamilyDescriptor] = [],
): RocksDBResult[TransactionDbRef] =
## Open a `TransactionDbRef` 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.
let useDbOpts = (if dbOpts.isNil: defaultDbOptions() else: dbOpts)
defer:
if dbOpts.isNil:
useDbOpts.close()

var cfs = columnFamilies.toSeq()
if DEFAULT_COLUMN_FAMILY_NAME notin columnFamilies.mapIt(it.name()):
Expand All @@ -64,7 +60,7 @@ proc openTransactionDb*(
errors: cstring

let txDbPtr = rocksdb_transactiondb_open_column_families(
useDbOpts.cPtr,
dbOpts.cPtr,
txDbOpts.cPtr,
path.cstring,
cfNames.len().cint,
Expand All @@ -76,10 +72,6 @@ proc openTransactionDb*(
bailOnErrors(errors)

let
dbOpts = useDbOpts # don't close on exit
txDbOpts = (if txDbOpts.isNil: defaultTransactionDbOptions()
else: txDbOpts
)
cfTable = newColFamilyTable(cfNames.mapIt($it), cfHandles)
db = TransactionDbRef(
lock: createLock(),
Expand Down Expand Up @@ -107,22 +99,15 @@ proc isClosed*(db: TransactionDbRef): bool {.inline.} =

proc beginTransaction*(
db: TransactionDbRef,
readOpts = ReadOptionsRef(nil),
writeOpts = WriteOptionsRef(nil),
txDbOpts = TransactionDbOptionsRef(nil),
readOpts = defaultReadOptions(),
writeOpts = defaultWriteOptions(),
txOpts = defaultTransactionOptions(),
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
txDbOpts = (if txDbOpts.isNil: defaultTransactionDbOptions()
else: txDbOpts
)
readOpts = (if readOpts.isNil: defaultReadOptions() else: readOpts)
writeOpts = (if writeOpts.isNil: defaultWriteOptions() else: writeOpts)

let txPtr = rocksdb_transaction_begin(db.cPtr, writeOpts.cPtr, txOpts.cPtr, nil)

Expand Down
1 change: 0 additions & 1 deletion tests/test_helper.nim
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ proc initTransactionDb*(
): TransactionDbRef =
let res = openTransactionDb(
path,
txDbOpts = defaultTransactionDbOptions(),
columnFamilies = columnFamilyNames.mapIt(initColFamilyDescriptor(it)),
)
if res.isErr():
Expand Down

0 comments on commit 953d9e8

Please sign in to comment.