Enhancements to the API #82
Conversation
|
LGTM I will probably switch my Go code to use gorocksdb instead of my gorocks package as soon as this is merged. |
|
Can you please add basic merge operator sanity tests to db/c_test.c (few puts, few gets, nothing major)? Can you also explain reason behind removing Otherwise looks good, thanks a lot! |
| std::string* new_value, | ||
| Logger* logger) const { | ||
|
|
||
| int n = operand_list.size(); |
There was a problem hiding this comment.
size_t n = operand_list.size();
:)
|
Hi Igor,
For the test I have currently no time. I could do it maybe on Friday. |
|
Somehow if the function returns a pointer the unwritten rule is that the caller is responsible for cleaning the memory. Can we make sure that client allocates the memory in the C space? Alternative solution could be to add a Delete method that would be default call free(filter), and client can implement its own solution for freeing the memory if he wishes. |
…make it possible to override the cleanup behaviour of the return value
|
I decided to implement your alternative solution also I added a test case for the merge operator. |
|
Tnx! |
|
Merge operator test seg faulted. Here's the fix: 2bf1151 |
|
Oh no, I didn't see the small message at the end of the test. Sry for that :/ |
Summary:
./db_compaction_test DBCompactionTest.LevelTtlCascadingCompactions passed 96 / 100 times.
```
With the fix: all runs (tried 100, 1000, 10000) succeed.
```
$ TEST_TMPDIR=/dev/shm ~/gtest-parallel/gtest-parallel ./db_compaction_test --gtest_filter=DBCompactionTest.LevelTtlCascadingCompactions --repeat=1000
[1000/1000] DBCompactionTest.LevelTtlCascadingCompactions (1895 ms)
```
Test Plan:
Build:
```
COMPILE_WITH_TSAN=1 make db_compaction_test -j100
```
Without the fix: a few runs out of 100 fail:
```
$ TEST_TMPDIR=/dev/shm KEEP_DB=1 ~/gtest-parallel/gtest-parallel ./db_compaction_test --gtest_filter=DBCompactionTest.LevelTtlCascadingCompactions --repeat=100
...
...
Note: Google Test filter = DBCompactionTest.LevelTtlCascadingCompactions
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from DBCompactionTest
[ RUN ] DBCompactionTest.LevelTtlCascadingCompactions
db/db_compaction_test.cc:3687: Failure
Expected equality of these values:
oldest_time
Which is: 1580155869
level_to_files[6][0].oldest_ancester_time
Which is: 1580155870
DB is still at /dev/shm//db_compaction_test_6337001442947696266
[ FAILED ] DBCompactionTest.LevelTtlCascadingCompactions (1432 ms)
[----------] 1 test from DBCompactionTest (1432 ms total)
[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (1433 ms total)
[ PASSED ] 0 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] DBCompactionTest.LevelTtlCascadingCompactions
1 FAILED TEST
[80/100] DBCompactionTest.LevelTtlCascadingCompactions returned/aborted with exit code 1 (1489 ms)
[100/100] DBCompactionTest.LevelTtlCascadingCompactions (1522 ms)
FAILED TESTS (4/100):
1419 ms: ./db_compaction_test DBCompactionTest.LevelTtlCascadingCompactions (try #90)
1434 ms: ./db_compaction_test DBCompactionTest.LevelTtlCascadingCompactions (try #84)
1457 ms: ./db_compaction_test DBCompactionTest.LevelTtlCascadingCompactions (try #82)
1489 ms: ./db_compaction_test DBCompactionTest.LevelTtlCascadingCompactions (try #74)
Differential Revision: D19587040
Pulled By: sagar0
fbshipit-source-id: 11191ae9940837643bff47ebe18b299b4be3d950
Separate out the configuration from initialization and construction
This pull request adds enhancements to the API which I need for my Go wrapper: https://github.com/tecbot/gorocksdb
Please review careful the part with the merge operator because I am not a C expert so maybe I did something wrong.
I completed the CLA.