Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose db stress tests #5937

Closed
wants to merge 1 commit into from

Conversation

riversand963
Copy link
Contributor

@riversand963 riversand963 commented Oct 18, 2019

Summary: expose db stress test by providing db_stress_tool.h in public header.
This PR does the following:

  • adds a new header, db_stress_tool.h, in include/rocksdb/
  • renames db_stress.cc to db_stress_tool.cc
  • adds a db_stress.cc which simply invokes a test function.
  • update Makefile accordingly.

Test Plan (dev server):

make db_stress
./db_stress

Summary: expose db stress test by providing db_stress_tool.h in public header.

Test Plan (dev server):
```
make db_stress
./db_stress
```
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

}
FLAGS_env->SetBackgroundThreads(new_thread_pool_size);
// Sleep up to 3 seconds
FLAGS_env->SleepForMicro
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the function defined if gflags isn't installed? If not the public header seems to reference to a function not implemented.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this is also the case for db_bench_tool?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK then...

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@riversand963 riversand963 deleted the expose_db_stress branch October 18, 2019 16:52
@facebook-github-bot
Copy link
Contributor

@riversand963 merged this pull request in e60cc09.

merryChris pushed a commit to merryChris/rocksdb that referenced this pull request Nov 18, 2019
Summary:
expose db stress test by providing db_stress_tool.h in public header.
This PR does the following:
- adds a new header, db_stress_tool.h, in include/rocksdb/
- renames db_stress.cc to db_stress_tool.cc
- adds a db_stress.cc which simply invokes a test function.
- update Makefile accordingly.

Test Plan (dev server):
```
make db_stress
./db_stress
```
Pull Request resolved: facebook#5937

Differential Revision: D17997647

Pulled By: riversand963

fbshipit-source-id: 1a8d9994f89ce198935566756947c518f0052410
@merryChris
Copy link

With WITH_TOOLS=ON, errors produces while building db_stress, since db_stress_tool.cc was not included. Even if added, it cannot work fine with db_bench.

@riversand963
Copy link
Contributor Author

@merryChris. Not very familiar with Cmake. Would you be willing to propose a fix for this?

@merryChris
Copy link

@merryChris. Not very familiar with Cmake. Would you be willing to propose a fix for this?

Fine, let me have a try.

@siying
Copy link
Contributor

siying commented Nov 26, 2019

@riversand963 I think new .cc files need to be added to both of src.md and CMakeList.txt.

@merryChris
Copy link

@riversand963 PR as it is. At first, I'd like to fix it by making flags isolation with namespace between db_bench_tool.cc and db_stress_tool.cc, which seems the easiest way. Unfortunately, it doesn't work, plz refer to gfalgs usage. After that, I HAVE TO abstract the common flags to solve flags conflicts. But got confusion while testing with db_stress. Two examples as follows.

Case#1:
./db_stress -prefix_size=0  -allow_concurrent_memtable_write=false

Output:
db_stress: /path/to/db_stress_tool.cc:3387: virtual rocksdb::Status rocksdb::NonBatchedOpsStressTest::TestPrefixScan(rocksdb::{anonymous}::ThreadState*, const rocksdb::ReadOptions&, const std::vector<int>&, const std::vector<long int>&): Assertion `count <= (static_cast<long>(1) << ((8 - FLAGS_prefix_size) * 8))' failed.
Aborted (core dumped)
Case#2
./db_stress -prefix_size=7 -memtablerep=prefix_hash -allow_concurrent_memtable_write=false

Output:
Iterator diverged from control iterator which has value 00000000000FEA36 seek key 00000000000FEAC6
iterator is not valid
Control CF 7
2019/11/27-14:00:00  Starting verification
Stress Test : 26.809 micros/op 37301 ops/sec
            : Wrote 0.03 MB (0.27 MB/sec) (46% of 4110 ops)
            : Wrote 1894 times
            : Deleted 611 times
            : Single deleted 0 times
            : 392 read and 0 found the key
            : Prefix scanned 806 times
            : Iterator size sum is 22
            : Iterated 690 times
            : Deleted 0 key-ranges
            : Range deletions covered 0 keys
            : Got errors 1 times
            : 0 CompactFiles() succeed
            : 0 CompactFiles() did not succeed
Verification failed :(

To be honest, I have to admit that I'm not familiar some flags. Could you plz give me a hand? At the same time, remind to review the PR. Thx in advance.

simpkins added a commit to simpkins/rocksdb that referenced this pull request Dec 3, 2019
Summary:
PR facebook#5937 changed the db_stress tool to also require db_stress_tool.cc,
and updated the Makefile but not the CMakeLists.txt file.  This updates
the CMakeLists.txt file so that the CMake build succeeds again.

PR facebook#5950 updated the Makefile build to package db_stress_tool.cc into
its own librocksdb_stress.a library.  I haven't done that here since
there didn't really seem to be much benefit: the Makefile-based build
does not install this library.

Test Plan:
Confirmed the CMake build succeeds on an Ubuntu 18.04 system.
facebook-github-bot pushed a commit that referenced this pull request Dec 5, 2019
Summary:
PR #5937 changed the db_stress tool to also require db_stress_tool.cc,
and updated the Makefile but not the CMakeLists.txt file.  This updates
the CMakeLists.txt file so that the CMake build succeeds again.

PR #5950 updated the Makefile build to package db_stress_tool.cc into
its own librocksdb_stress.a library.  I haven't done that here since
there didn't really seem to be much benefit: the Makefile-based build
does not install this library.
Pull Request resolved: #6117

Test Plan: Confirmed the CMake build succeeds on an Ubuntu 18.04 system.

Differential Revision: D18835053

Pulled By: riversand963

fbshipit-source-id: 6e2a66834716e73b1eb736d9b7159870defffec5
wolfkdy pushed a commit to wolfkdy/rocksdb that referenced this pull request Dec 22, 2019
…ok#6117)

Summary:
PR facebook#5937 changed the db_stress tool to also require db_stress_tool.cc,
and updated the Makefile but not the CMakeLists.txt file.  This updates
the CMakeLists.txt file so that the CMake build succeeds again.

PR facebook#5950 updated the Makefile build to package db_stress_tool.cc into
its own librocksdb_stress.a library.  I haven't done that here since
there didn't really seem to be much benefit: the Makefile-based build
does not install this library.
Pull Request resolved: facebook#6117

Test Plan: Confirmed the CMake build succeeds on an Ubuntu 18.04 system.

Differential Revision: D18835053

Pulled By: riversand963

fbshipit-source-id: 6e2a66834716e73b1eb736d9b7159870defffec5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants