Tag:
Branch:
Tree:
39c29372bf
main
oxigraph-8.1.1
oxigraph-8.3.2
oxigraph-main
${ noResults }
11820 Commits (39c29372bfc77cab48f0d4bddc5fdf1cec47e577)
| Author | SHA1 | Message | Date |
|---|---|---|---|
Levi Tamasi
|
a82021c3d0 |
Fix a bug where GetEntity would expose a blob reference (#11162)
Summary: The patch fixes a feature interaction bug between BlobDB and the `GetEntity` API: without the patch, `GetEntity` would return the blob reference (wrapped into a single-column entity) instead of the actual blob value. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11162 Test Plan: `make check` Reviewed By: akankshamahajan15 Differential Revision: D42854092 Pulled By: ltamasi fbshipit-source-id: f750d0ff57def107da16f545077ddce9860ff21a |
2 years ago |
|
Peter Dillinger
|
94e3beec77 |
Cleanup, improve, stress test LockWAL() (#11143)
Summary: The previous API comments for LockWAL didn't provide much about why you might want to use it, and didn't really meet what one would infer its contract was. Also, LockWAL was not in db_stress / crash test. In this change: * Implement a counting semantics for LockWAL()+UnlockWAL(), so that they can safely be used concurrently across threads or recursively within a thread. This should make the API much less bug-prone and easier to use. * Make sure no UnlockWAL() is needed after non-OK LockWAL() (to match RocksDB conventions) * Make UnlockWAL() reliably return non-OK when there's no matching LockWAL() (for debug-ability) * Clarify API comments on LockWAL(), UnlockWAL(), FlushWAL(), and SyncWAL(). Their exact meanings are not obvious, and I don't think it's appropriate to talk about implementation mutexes in the API comments, but about what operations might block each other. * Add LockWAL()/UnlockWAL() to db_stress and crash test, mostly to check for assertion failures, but also checks that latest seqno doesn't change while WAL is locked. This is simpler to add when LockWAL() is allowed in multiple threads. * Remove unnecessary use of sync points in test DBWALTest::LockWal. There was a bug during development of above changes that caused this test to fail sporadically, with and without this sync point change. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11143 Test Plan: unit tests added / updated, added to stress/crash test Reviewed By: ajkr Differential Revision: D42848627 Pulled By: pdillinger fbshipit-source-id: 6d976c51791941a31fd8fbf28b0f82e888d9f4b4 |
2 years ago |
|
sdong
|
36174d89a6 |
DB Stress to fix a false assertion (#11164)
Summary: Seeting this error in stress test: db_stress: internal_repo_rocksdb/repo/db_stress_tool/db_stress_test_base.cc:2459: void rocksdb::StressTest::Open(rocksdb::SharedState *): Assertion `txn_db_ == nullptr' failed. Received signal 6 (Aborted) ...... It doesn't appear that txn_db_ is set to nullptr at all. We set ithere. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11164 Test Plan: Run db_stress transaction and non-transation with low kill rate and see restarting without assertion Reviewed By: ajkr Differential Revision: D42855662 fbshipit-source-id: 06816d37cce9c94a81cb54ab238fb73aa102ed46 |
2 years ago |
|
Yu Zhang
|
24ac53d81a |
Use user key on sst file for blob verification for Get and MultiGet (#11105)
Summary: Use the user key on sst file for blob verification for `Get` and `MultiGet` instead of the user key passed from caller. Add tests for `Get` and `MultiGet` operations when user defined timestamp feature is enabled in a BlobDB. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11105 Test Plan: make V=1 db_blob_basic_test ./db_blob_basic_test --gtest_filter="DBBlobTestWithTimestamp.*" Reviewed By: ltamasi Differential Revision: D42716487 Pulled By: jowlyzhang fbshipit-source-id: 5987ecbb7e56ddf46d2467a3649369390789506a |
2 years ago |
|
akankshamahajan
|
79e57a39a3 |
Move ExternalSSTTestEnv to FileSystemWrapper (#11139)
Summary: Migrate ExternalSSTTestEnv to FileSystemWrapper Pull Request resolved: https://github.com/facebook/rocksdb/pull/11139 Reviewed By: anand1976 Differential Revision: D42780180 Pulled By: akankshamahajan15 fbshipit-source-id: 9a4448c9fe5186b518235fe11e1a34dcad897cdd |
2 years ago |
|
sdong
|
4720ba4391 |
Remove RocksDB LITE (#11147)
Summary: We haven't been actively mantaining RocksDB LITE recently and the size must have been gone up significantly. We are removing the support. Most of changes were done through following comments: unifdef -m -UROCKSDB_LITE `git grep -l ROCKSDB_LITE | egrep '[.](cc|h)'` by Peter Dillinger. Others changes were manually applied to build scripts, CircleCI manifests, ROCKSDB_LITE is used in an expression and file db_stress_test_base.cc. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11147 Test Plan: See CI Reviewed By: pdillinger Differential Revision: D42796341 fbshipit-source-id: 4920e15fc2060c2cd2221330a6d0e5e65d4b7fe2 |
2 years ago |
|
Yu Zhang
|
6943ff6e50 |
Remove deprecated util functions in options_util.h (#11126)
Summary: Remove the util functions in options_util.h that have previously been marked deprecated. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11126 Test Plan: `make check` Reviewed By: ltamasi Differential Revision: D42757496 Pulled By: jowlyzhang fbshipit-source-id: 2a138a3c207d0e0e0bbb4d99548cf2cadb44bcfb |
2 years ago |
|
Andrew Kryczka
|
97c1024d3e |
Include db_stress verification method in failure message (#11133)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/11133 Test Plan: - ran it a few times on a mismatching DB+expected state; verified error messages look right: ``` Verification failed for column family 0 key 000000000000D553000000000000014C0000000000000142 (163988): value_from_db: , value_from_expected: 25E7B53421202322, msg: GetMergeOperands verification: Value not found: NotFound: Verification failed for column family 0 key 000000000000AAE2787878 (131123): value_from_db: , value_from_expected: B2A69C18B6B7B4B5BABBB8B9BEBFBCBDA2A3A0A1A6A7A4A5, msg: Iterator verification: Value not found: NotFound: Verification failed for column family 0 key 00000000000080C6000000000000004C78787878 (98409): value_from_db: , value_from_expected: 67AB7E1E636261606F6E6D6C6B6A6968, msg: Get verification: Value not found: NotFound: ``` Reviewed By: hx235 Differential Revision: D42757072 Pulled By: ajkr fbshipit-source-id: b0a4a0aaa5be5d110434324853ac92aaa6972d89 |
2 years ago |
|
Changyu Bi
|
c94c8fcbd4 |
Remove deprecated FileSystem::Load() (#11122)
Summary: user should use FileSystem::CreateFromString() instead. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11122 Reviewed By: ajkr Differential Revision: D42727580 Pulled By: cbi42 fbshipit-source-id: c68b17bb82ba9dee46ba23b677d87ecf0a1e06c8 |
2 years ago |
|
Karim TAAM
|
a1e92bd956 |
use verify checksum option in block based table reader Open() (#11099)
Summary: ## Description In this issue https://github.com/facebook/rocksdb/issues/11002 we found that when we use rocksdb with the `verify checksum` read_option to false the verification is done anyway By analyzing the code along the stacktrace I saw that at the level of https://github.com/facebook/rocksdb/compare/main...matkt:feature/use-verify-checksum-in-block-based-table-reader?expand=1#diff-57ed8c49db2bdd4db7618646a177397674bbf25beacacecb104070071d30129f we are not keeping all the options and we forget the `verify_checksum` the comment in this class suggests that it should be managed https://github.com/facebook/rocksdb/compare/main...matkt:feature/use-verify-checksum-in-block-based-table-reader?expand=1#diff-57ed8c49db2bdd4db7618646a177397674bbf25beacacecb104070071d30129fL581 <img width="1724" alt="204511641-86ab4b9b-45e5-4a2b-a13d-81fa26435d38" src="https://user-images.githubusercontent.com/26581503/213152802-c46bc1c7-a3a2-4a6f-9bb1-bf92ee93af7a.png"> this PR just adds the line to manage the `verify checksum` ## Tests - Running unit tests - Test without setting `verify checksum` and verifying that we are calling the checksum code - Test by setting `verify checksum` to true and verifying that we are calling the checksum code - Test by setting `verify checksum` to false and verifying that we are **not** calling the checksum code Pull Request resolved: https://github.com/facebook/rocksdb/pull/11099 Reviewed By: cbi42 Differential Revision: D42679881 Pulled By: ajkr fbshipit-source-id: c7dd10768282fd0699f7e1bf397ceb7adbea4ab6 |
2 years ago |
|
Andrew Kryczka
|
b44cbbf709 |
Fix GetMergeOperands() returning MergeInProgress (#11136)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/11136 Test Plan: the provided unit test used to fail due to `GetMergeOperands()` returning `Status::MergeInProgress()`; it passes now because the `GetMergeOperands()` call returns `Status::OK()` Reviewed By: pdillinger Differential Revision: D42759198 Pulled By: ajkr fbshipit-source-id: 878f9f40ccc1d7e2fe7b1352814bae3a49c19939 |
2 years ago |
|
dependabot[bot]
|
dcf93b7b3e |
Bump commonmarker from 0.23.6 to 0.23.7 in /docs (#11128)
Summary: Bumps [commonmarker](https://github.com/gjtorikian/commonmarker) from 0.23.6 to 0.23.7. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/gjtorikian/commonmarker/releases">commonmarker's releases</a>.</em></p> <blockquote> <h2>v0.23.7</h2> <h2>What's Changed</h2> <ul> <li>C API stable test by <a href="https://github.com/gjtorikian"><code>@gjtorikian</code></a> in <a href="https://github-redirect.dependabot.com/gjtorikian/commonmarker/pull/201">gjtorikian/commonmarker#201</a></li> <li>Update to 29.0.gfm.7 by <a href="https://github.com/anticomputer"><code>@anticomputer</code></a> in <a href="https://github-redirect.dependabot.com/gjtorikian/commonmarker/pull/224">gjtorikian/commonmarker#224</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/gjtorikian/commonmarker/compare/v0.23.6...v0.23.7">https://github.com/gjtorikian/commonmarker/compare/v0.23.6...v0.23.7</a></p> <h2>v0.23.7.pre1</h2> <h2>What's Changed</h2> <ul> <li>C API stable test by <a href="https://github.com/gjtorikian"><code>@gjtorikian</code></a> in <a href="https://github-redirect.dependabot.com/gjtorikian/commonmarker/pull/201">gjtorikian/commonmarker#201</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/gjtorikian/commonmarker/compare/v0.23.6...v0.23.7.pre1">https://github.com/gjtorikian/commonmarker/compare/v0.23.6...v0.23.7.pre1</a></p> </blockquote> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/gjtorikian/commonmarker/blob/main/CHANGELOG.md">commonmarker's changelog</a>.</em></p> <blockquote> <h1>Changelog</h1> <h2><a href="https://github.com/gjtorikian/commonmarker/tree/v1.0.0.pre6">v1.0.0.pre6</a> (2023-01-09)</h2> <p><a href="https://github.com/gjtorikian/commonmarker/compare/v1.0.0.pre5...v1.0.0.pre6">Full Changelog</a></p> <p><strong>Closed issues:</strong></p> <ul> <li>Cargo.lock prevents Ruby 3.2.0 from installing commonmarker v1.0.0.pre4 <a href="https://github-redirect.dependabot.com/gjtorikian/commonmarker/issues/211">https://github.com/facebook/rocksdb/issues/211</a></li> </ul> <p><strong>Merged pull requests:</strong></p> <ul> <li>always use rb_sys (don't use Ruby's emerging cargo tooling where available) <a href="https://github-redirect.dependabot.com/gjtorikian/commonmarker/pull/213">https://github.com/facebook/rocksdb/issues/213</a> (<a href="https://github.com/kivikakk">kivikakk</a>)</li> </ul> <h2><a href="https://github.com/gjtorikian/commonmarker/tree/v1.0.0.pre5">v1.0.0.pre5</a> (2023-01-08)</h2> <p><a href="https://github.com/gjtorikian/commonmarker/compare/v1.0.0.pre4...v1.0.0.pre5">Full Changelog</a></p> <p><strong>Merged pull requests:</strong></p> <ul> <li>Provide 3.2 build support <a href="https://github-redirect.dependabot.com/gjtorikian/commonmarker/pull/212">https://github.com/facebook/rocksdb/issues/212</a> (<a href="https://github.com/gjtorikian">gjtorikian</a>)</li> </ul> <h2><a href="https://github.com/gjtorikian/commonmarker/tree/v1.0.0.pre4">v1.0.0.pre4</a> (2022-12-28)</h2> <p><a href="https://github.com/gjtorikian/commonmarker/compare/v1.0.0.pre3...v1.0.0.pre4">Full Changelog</a></p> <p><strong>Closed issues:</strong></p> <ul> <li>Will the cmark-gfm branch continue to be maintained for awhile? <a href="https://github-redirect.dependabot.com/gjtorikian/commonmarker/issues/207">https://github.com/facebook/rocksdb/issues/207</a></li> </ul> <p><strong>Merged pull requests:</strong></p> <ul> <li>Implement native syntax highlighting <a href="https://github-redirect.dependabot.com/gjtorikian/commonmarker/pull/209">https://github.com/facebook/rocksdb/issues/209</a> (<a href="https://github.com/gjtorikian">gjtorikian</a>)</li> <li>Bump magnus from 0.4.3 to 0.4.4 <a href="https://github-redirect.dependabot.com/gjtorikian/commonmarker/pull/208">https://github.com/facebook/rocksdb/issues/208</a> (<a href="https://github.com/apps/dependabot">dependabot[bot]</a>)</li> <li>Bump magnus from 0.4.2 to 0.4.3 <a href="https://github-redirect.dependabot.com/gjtorikian/commonmarker/pull/206">https://github.com/facebook/rocksdb/issues/206</a> (<a href="https://github.com/apps/dependabot">dependabot[bot]</a>)</li> <li>Bump comrak from 0.14.0 to 0.15.0 <a href="https://github-redirect.dependabot.com/gjtorikian/commonmarker/pull/205">https://github.com/facebook/rocksdb/issues/205</a> (<a href="https://github.com/apps/dependabot">dependabot[bot]</a>)</li> <li>Bump magnus from 0.4.1 to 0.4.2 <a href="https://github-redirect.dependabot.com/gjtorikian/commonmarker/pull/204">https://github.com/facebook/rocksdb/issues/204</a> (<a href="https://github.com/apps/dependabot">dependabot[bot]</a>)</li> </ul> <h2><a href="https://github.com/gjtorikian/commonmarker/tree/v1.0.0.pre3">v1.0.0.pre3</a> (2022-11-30)</h2> <p><a href="https://github.com/gjtorikian/commonmarker/compare/v1.0.0.pre.2...v1.0.0.pre3">Full Changelog</a></p> <p><strong>Closed issues:</strong></p> <ul> <li>Code block incorrectly parsed in commonmarker 1.0.0.pre <a href="https://github-redirect.dependabot.com/gjtorikian/commonmarker/issues/202">https://github.com/facebook/rocksdb/issues/202</a></li> </ul> <p><strong>Merged pull requests:</strong></p> <ul> <li>Windows build <a href="https://github-redirect.dependabot.com/gjtorikian/commonmarker/pull/197">https://github.com/facebook/rocksdb/issues/197</a> (<a href="https://github.com/gjtorikian">gjtorikian</a>)</li> </ul> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href=" |
2 years ago |
|
Levi Tamasi
|
a6cfdd4eda |
Fix the HISTORY.md entry related to the removed statistics (#11140)
Summary: Some histograms were incorrectly categorized as tickers. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11140 Reviewed By: anand1976 Differential Revision: D42780030 Pulled By: ltamasi fbshipit-source-id: 5aca8ec5baad8f73676aaa9d6cdbbd2a619c8a89 |
2 years ago |
|
akankshamahajan
|
986c5b9d4e |
Migrate TestEnv in listener_test.cc to FileSystemWrapper (#11125)
Summary: Migrate derived classes from EnvWrapper to FileSystemWrapper so we can eventually deprecate the storage methods in Env. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11125 Test Plan: CircleCI jobs Reviewed By: anand1976 Differential Revision: D42732241 Pulled By: akankshamahajan15 fbshipit-source-id: c89a70a79fcfb13e158bf8919b1a87a9de133222 |
2 years ago |
|
sdong
|
e808858ae0 |
Remove Stats related to compressed block cache (#11135)
Summary: Since compressed block cache is removed, those stats are not needed. They are removed in different PR in case there is a problem with it. The stats are removed in the same way in https://github.com/facebook/rocksdb/pull/11131/ . HISTORY.md was already updated by mistake, and it would be correct after merging this PR. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11135 Test Plan: Watch CI Reviewed By: ltamasi Differential Revision: D42757616 fbshipit-source-id: bd7cb782585c8535ce5784295225c376f3011f35 |
2 years ago |
|
Levi Tamasi
|
6da2e20df3 |
Remove more obsolete statistics (#11131)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/11131 Test Plan: `make check` Reviewed By: pdillinger Differential Revision: D42753997 Pulled By: ltamasi fbshipit-source-id: ce8b84c1e55374257e93ed74fd255c9b759723ce |
2 years ago |
|
Heiko Becker
|
88edfbfb5e |
Fix build with gcc 13 by including <cstdint> (#11118)
Summary:
Like other versions before, gcc 13 moved some includes around and as a result <cstdint> is no longer transitively included [1]. Explicitly include it for uint{32,64}_t.
[1] https://gcc.gnu.org/gcc-13/porting_to.html#header-dep-changes
Pull Request resolved: https://github.com/facebook/rocksdb/pull/11118
Reviewed By: cbi42
Differential Revision: D42711356
Pulled By: ajkr
fbshipit-source-id: 5ea257b85b7017f40fd8fdbce965336da95c55b2
|
2 years ago |
|
Andrew Kryczka
|
6a5071ceb5 |
Support PutEntity in trace analyzer (#11127)
Summary: Add the most basic support such that trace_analyzer commands no longer fail with ``` Cannot process the write batch in the trace Cannot process the TraceRecord PutEntityCF not implemented Cannot process the trace ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/11127 Reviewed By: cbi42 Differential Revision: D42732319 Pulled By: ajkr fbshipit-source-id: 162d8a31318672a46539b1b042ec25f69b25c4ed |
2 years ago |
|
Peter Dillinger
|
546e213c4f |
Fix DelayWrite() calls for two_write_queues (#11130)
Summary: PR https://github.com/facebook/rocksdb/issues/11020 fixed a case where it was easy to deadlock the DB with LockWAL() but introduced a bug showing up as a rare assertion failure in the stress test. Specifically, `assert(w->state == STATE_INIT)` in `WriteThread::LinkOne()` called from `BeginWriteStall()`, `DelayWrite()`, `WriteImplWALOnly()`. I haven't been about to generate a unit test that reproduces this failure but I believe the root cause is that DelayWrite() was never meant to be re-entrant, only called from the DB's write_thread_ leader. https://github.com/facebook/rocksdb/issues/11020 introduced a call to DelayWrite() from the nonmem_write_thread_ group leader. This fix is to make DelayWrite() apply to the specific write queue that it is being called from (inject a dummy write stall entry to the head of the appropriate write queue). WriteController is re-entrant, based on polling and state changes signalled with bg_cv_, so can manage stalling two queues. The only anticipated complication (called out by Andrew in previous PR) is that we don't want timed write delays being injected in parallel for the two queues, because that dimishes the intended throttling effect. Thus, we only allow timed delays for the primary write queue. HISTORY not updated because this is intended for the same release where the bug was introduced. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11130 Test Plan: Although I was not able to reproduce the assertion failure, I was able to reproduce a distinct flaw with what I believe is the same root cause: a kind of deadlock if both write queues need to wake up from stopped writes. Only one will be waiting on bg_cv_ (the other waiting in `LinkOne()` for the write queue to open up), so a single SignalAll() will only unblock one of the queues, with the other re-instating the stop until another signal on bg_cv_. A simple unit test is added for this case. Will also run crash_test_with_multiops_wc_txn for a while looking for issues. Reviewed By: ajkr Differential Revision: D42749330 Pulled By: pdillinger fbshipit-source-id: 4317dd899a93d57c26fd5af7143038f82d4d4d1b |
2 years ago |
|
Peter Dillinger
|
9afa0f05ad |
Remove deprecated Env::LoadEnv() (#11121)
Summary: Can use Env::CreateFromString() instead Pull Request resolved: https://github.com/facebook/rocksdb/pull/11121 Test Plan: unit tests updated Reviewed By: cbi42 Differential Revision: D42723813 Pulled By: pdillinger fbshipit-source-id: 5d4b5b10225dfdaf662f5f8049ee965a05d3edc9 |
2 years ago |
|
Levi Tamasi
|
99e559533d |
Remove some deprecated/obsolete statistics from the API (#11123)
Summary: These tickers/histograms have been obsolete (and not populated) for a long time. The patch removes them from the API completely. Note that this means that the numeric values of the remaining tickers change in the C++ code as they get shifted up. This should be OK: the values of some existing tickers have changed many times over the years as items have been added in the middle. (In contrast, the convention in the Java bindings is to keep the ids, which are not guaranteed to be the same as the ids on the C++ side, the same across releases.) Pull Request resolved: https://github.com/facebook/rocksdb/pull/11123 Test Plan: `make check` Reviewed By: akankshamahajan15 Differential Revision: D42727793 Pulled By: ltamasi fbshipit-source-id: e058a155a20b05b45f53e67ee380aece1b43b6c5 |
2 years ago |
|
anand76
|
bcbab59c55 |
Migrate ErrorEnv from EnvWrapper to FileSystemWrapper (#11124)
Summary: Migrate ErrorEnv from EnvWrapper to FileSystemWrapper so we can eventually deprecate the storage methods in Env. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11124 Reviewed By: akankshamahajan15 Differential Revision: D42727791 Pulled By: anand1976 fbshipit-source-id: e8362ad624dc28e55c99fc35eda12866755f62c6 |
2 years ago |
|
sdong
|
2800aa069a |
Remove compressed block cache (#11117)
Summary: Compressed block cache is replaced by compressed secondary cache. Remove the feature. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11117 Test Plan: See CI passes Reviewed By: pdillinger Differential Revision: D42700164 fbshipit-source-id: 6cbb24e460da29311150865f60ecb98637f9f67d |
2 years ago |
|
Peter Dillinger
|
4a9185340d |
A better contract for best_efforts_recovery (#11085)
Summary: Capture more of the original intent at a high level, without getting bogged down in low-level details. The old text made some weak promises about handling of LOCK files. There should be no specific concern for LOCK files, because we already rely on LockFile() to create the file if it's not present already. And the lock file is generally size 0, so don't have to worry about truncation. Added a unit test. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11085 Test Plan: existing tests, and a new one. Reviewed By: siying Differential Revision: D42713233 Pulled By: pdillinger fbshipit-source-id: 2fce7c974d35fac065037c9c4c7326a59c9fe340 |
2 years ago |
|
Changyu Bi
|
e0ea0dc6bd |
Improve documentation for `allow_ingest_behind` (#11119)
Summary: update documentation to mention that only universal compaction is supported. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11119 Reviewed By: ajkr Differential Revision: D42715986 Pulled By: cbi42 fbshipit-source-id: 91b145d3318334cb92857c5c0ffc0efed6fa4363 |
2 years ago |
|
Hui Xiao
|
86fa2592be |
Fix data race on `ColumnFamilyData::flush_reason` by letting FlushRequest/Job owns flush_reason instead of CFD (#11111)
Summary: **Context:** Concurrent flushes on the same CF can set on `ColumnFamilyData::flush_reason` before each other flush finishes. An symptom is one CF has different flush_reason with others though all of them are in an atomic flush `db_stress: db/db_impl/db_impl_compaction_flush.cc:423: rocksdb::Status rocksdb::DBImpl::AtomicFlushMemTablesToOutputFiles(const rocksdb::autovector<rocksdb::DBImpl::BGFlushArg>&, bool*, rocksdb::JobContext*, rocksdb::LogBuffer*, rocksdb::Env::Priority): Assertion cfd->GetFlushReason() == cfds[0]->GetFlushReason() failed. ` **Summary:** Suggested by ltamasi, we now refactor and let FlushRequest/Job to own flush_reason as there is no good way to define `ColumnFamilyData::flush_reason` in face of concurrent flushes on the same CF (which wasn't the case a long time ago when `ColumnFamilyData::flush_reason ` first introduced`) **Tets:** - new unit test - make check - aggressive crash test rehearsal Pull Request resolved: https://github.com/facebook/rocksdb/pull/11111 Reviewed By: ajkr Differential Revision: D42644600 Pulled By: hx235 fbshipit-source-id: 8589c8184869d3415e5b780c887f877818a5ebaf |
2 years ago |
|
Hui Xiao
|
7e7548477c |
Update HISTORY.md/version.h/format compatiblity test for 7.10 release (#11114)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/11114 Reviewed By: ajkr Differential Revision: D42685234 Pulled By: hx235 fbshipit-source-id: 79908a66ab9052a2552f080049065462ebf2f94c |
2 years ago |
|
Andrew Kryczka
|
b7fbcefda8 |
Add API to limit blast radius of merge operator failure (#11092)
Summary: Prior to this PR, `FullMergeV2()` can only return `false` to indicate failure, which causes any operation invoking it to fail. During a compaction, such a failure causes the compaction to fail and causes the DB to irreversibly enter read-only mode. Some users asked for a way to allow the merge operator to fail without such widespread damage. To limit the blast radius of merge operator failures, this PR introduces the `MergeOperationOutput::op_failure_scope` API. When unpopulated (`kDefault`) or set to `kTryMerge`, the merge operator failure handling is the same as before. When set to `kMustMerge`, merge operator failure still causes failure to operations that must merge (`Get()`, iterator, `MultiGet()`, etc.). However, under `kMustMerge`, flushes/compactions can survive merge operator failures by outputting the unmerged input operands. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11092 Reviewed By: siying Differential Revision: D42525673 Pulled By: ajkr fbshipit-source-id: 951dc3bf190f86347dccf3381be967565cda52ee |
2 years ago |
|
akankshamahajan
|
bde65052c4 |
Enhance async scan prefetch unit tests (#11087)
Summary: Add more coverage in unit tests for async scan. The added unit test fails without PR https://github.com/facebook/rocksdb/pull/10939. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11087 Test Plan: CircleCI jobs status for new unit tests. Reviewed By: anand1976 Differential Revision: D42487931 Pulled By: akankshamahajan15 fbshipit-source-id: d59ed7666599bd0d2733ac5d76bd70984b54c5a9 |
2 years ago |
|
codeoos
|
f4a5446cab |
Fix error maybe-uninitialized #11100 (#11101)
Summary: In this issue [11100](https://github.com/facebook/rocksdb/issues/11100) I try to upgrade dependencies of [BaikalDB](https://github.com/baidu/BaikalDB) and tool chain to gcc-12.I found that when I build rocksdb v6.26.0(maybe I can use newer version),I found that in file trace_replay/trace_replay.cc,the compiler tell me "error mybe-uninitialized".I dound that it can be fixed very easy,so I make this pull request. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11101 Reviewed By: ajkr Differential Revision: D42583031 Pulled By: cbi42 fbshipit-source-id: 7f399f09441a30fe88b83cec5e2fd9885bad5c06 |
2 years ago |
|
leipeng
|
a5bcbcd8be |
remove unused InternalIteratorBase::is_mutable_ (#11104)
Summary: `InternalIteratorBase::is_mutable_` is not used any more, remove it. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11104 Reviewed By: ajkr Differential Revision: D42582747 Pulled By: cbi42 fbshipit-source-id: d30bf75151fc8414df0ae112a6ec4943b5b7330b |
2 years ago |
|
Peter Dillinger
|
fd911f9655 |
Upgrade xxhash.h to latest dev (#11098)
Summary:
Upgrading xxhash.h to latest dev version as of 1/17/2023, which is d7197ddea81364a539051f116ca77926100fc77f This should improve performance on some ARM machines.
I allowed some of our RocksDB-specific changes to be made obsolete where it seemed appropriate, for example
* xxhash.h has its own fallthrough marker (which I hope works for us)
* As in https://github.com/Cyan4973/xxHash/pull/549
Merging and resolving conflicts one way or the other was all that went into this diff. Except I had to mix the two sides around `defined(__loongarch64)`
How I did the upgrade (for future reference), so that I could use usual merge conflict resolution:
```
# New branch to help with merging
git checkout -b xxh_merge_base
# Check out RocksDB revision before last xxhash.h upgrade
git reset --hard 22161b7547652af82a5dc67458de9ca8946ac83d^
# Create a commit with the raw base version from xxHash repo (from xxHash repo)
git show 2c611a76f914828bed675f0f342d6c4199ffee1e:xxhash.h > ../rocksdb/util/xxhash.h
# In RocksDB repo
git commit -a
# Merge in the last xxhash.h upgrade
git merge
|
2 years ago |
|
Changyu Bi
|
e9d6a0d7ce |
Fix asan failure caused by range tombstone start key use-after-free (#11106)
Summary: the `last_tombstone_start_user_key` variable in `BuildTable()` and in `CompactionOutputs::AddRangeDels()` may point to a start key that is freed if user-defined timestamp is enabled. This was causing ASAN failure and this PR fixes this issue. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11106 Test Plan: Added UT for repro. Reviewed By: ajkr Differential Revision: D42590862 Pulled By: cbi42 fbshipit-source-id: c493265ececdf89636d801d55ae929806c4d4b2c |
2 years ago |
|
akankshamahajan
|
bd4b8d6487 |
Fix crash in block_cache_trace_analyzer if reference key is null in case of MultiGet (#11042)
Summary: Same as title Error: ``` block_cache_trace_analyzer: ./db/dbformat.h:421: uint64_t rocksdb::GetInternalKeySeqno(const rocksdb::Slice&): Assertion `n >= kNumInternalBytes' failed. Aborted (core dumped) ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/11042 Test Plan: - Added new unit test which fails without the fix. - Also ran manually on traces to confirm. Reviewed By: anand1976 Differential Revision: D42481587 Pulled By: akankshamahajan15 fbshipit-source-id: 7c33eb03a4a4d8ffbabcfbe0efa1e4d11bde3ba2 |
2 years ago |
|
Changyu Bi
|
4d0f9a995c |
Consider TTL compaction file cutting earlier to prevent small output file (#11075)
Summary: in `CompactionOutputs::ShouldStopBefore()`, TTL-related states, `cur_files_to_cut_for_ttl_` and `next_files_to_cut_for_ttl_`, are not updated if the function returns early. This can cause unnecessary compaction output file cuttings and hence produce smaller output files, which may hurt write amp. See the example in the unit test for how this "unnecessary file cutting" can happen. This PR fixes this issue by moving the code for updating TTL states earlier in `CompactionOutputs::ShouldStopBefore()` so that the states are updated for each key. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11075 Test Plan: - Added new unit test. Reviewed By: hx235 Differential Revision: D42398739 Pulled By: cbi42 fbshipit-source-id: 09fab66679c1a734abcfc31bcea33dd9aeb9dbc7 |
2 years ago |
|
Changyu Bi
|
6a82b68788 |
Avoid counting extra range tombstone compensated size in `AddRangeDels()` (#11091)
Summary: in `CompactionOutputs::AddRangeDels()`, range tombstones with the same start and end key but different sequence numbers all contribute to compensated range tombstone size. This PR removes this redundancy. This PR also includes a fix from https://github.com/facebook/rocksdb/issues/11067 where a range tombstone that is not within a file's range was being added to the file. This fixes an assertion failure for `icmp.Compare(start, end) <= 0` in VersionSet::ApproximateSize() when calculating compensated range tombstone size. Assertions and a comment/essay was added to reason that no such range tombstone will be added after this fix. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11091 Test Plan: - Added unit tests - Stress test with small key range: `python3 tools/db_crashtest.py blackbox --simple --max_key=100 --interval=600 --write_buffer_size=262144 --target_file_size_base=256 --max_bytes_for_level_base=262144 --block_size=128 --value_size_mult=33 --subcompactions=10` Reviewed By: ajkr Differential Revision: D42521588 Pulled By: cbi42 fbshipit-source-id: 5bda3fe38997995314e1f7592319af12b69bc4f8 |
2 years ago |
|
Changyu Bi
|
f515d9d203 |
Revert #10802 Consider range tombstone in compaction output file cutting (#11089)
Summary:
This reverts commit
|
2 years ago |
|
leipeng
|
3941c34950 |
db_bench: let -benchmark=compact respect -subcompactions (#11077)
Summary: When running `-benchmarks=compact`, `-subcompactions` does not take effect. `-subcompactions` option comment says it is for L0-L1 compactions, it is natural to extend it to CompactionRangeOptions.max_subcompactions. This PR set CompactionRangeOptions.max_subcompactions = FLAGS_subcompactions Pull Request resolved: https://github.com/facebook/rocksdb/pull/11077 Reviewed By: akankshamahajan15 Differential Revision: D42506251 Pulled By: ajkr fbshipit-source-id: f77c9a99d32ff7af59f3c452c9e16aaeb0360304 |
2 years ago |
|
Wenlong Zhang
|
1cfe3528a2 |
support loongarch64 for rocksdb (#10036)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/10036 Reviewed By: hx235 Differential Revision: D42424074 Pulled By: ajkr fbshipit-source-id: 004adb75005a26bd01c5d568d1ec6ac442cd59dd |
2 years ago |
|
anand76
|
a510880346 |
Add a unit test for async prefetch fix in #11049 (#11084)
Summary: Add a unit test in prefetch_test for https://github.com/facebook/rocksdb/issues/11049 Pull Request resolved: https://github.com/facebook/rocksdb/pull/11084 Test Plan: Verify the test fails without https://github.com/facebook/rocksdb/issues/11049 and passes with it Reviewed By: akankshamahajan15 Differential Revision: D42485828 Pulled By: anand1976 fbshipit-source-id: ae512f2d121745a1f5212645a9b58868976c1f83 |
2 years ago |
|
Peter Dillinger
|
9f7801c5f1 |
Major Cache refactoring, CPU efficiency improvement (#10975)
Summary: This is several refactorings bundled into one to avoid having to incrementally re-modify uses of Cache several times. Overall, there are breaking changes to Cache class, and it becomes more of low-level interface for implementing caches, especially block cache. New internal APIs make using Cache cleaner than before, and more insulated from block cache evolution. Hopefully, this is the last really big block cache refactoring, because of rather effectively decoupling the implementations from the uses. This change also removes the EXPERIMENTAL designation on the SecondaryCache support in Cache. It seems reasonably mature at this point but still subject to change/evolution (as I warn in the API docs for Cache). The high-level motivation for this refactoring is to minimize code duplication / compounding complexity in adding SecondaryCache support to HyperClockCache (in a later PR). Other benefits listed below. * static_cast lines of code +29 -35 (net removed 6) * reinterpret_cast lines of code +6 -32 (net removed 26) ## cache.h and secondary_cache.h * Always use CacheItemHelper with entries instead of just a Deleter. There are several motivations / justifications: * Simpler for implementations to deal with just one Insert and one Lookup. * Simpler and more efficient implementation because we don't have to track which entries are using helpers and which are using deleters * Gets rid of hack to classify cache entries by their deleter. Instead, the CacheItemHelper includes a CacheEntryRole. This simplifies a lot of code (cache_entry_roles.h almost eliminated). Fixes https://github.com/facebook/rocksdb/issues/9428. * Makes it trivial to adjust SecondaryCache behavior based on kind of block (e.g. don't re-compress filter blocks). * It is arguably less convenient for many direct users of Cache, but direct users of Cache are now rare with introduction of typed_cache.h (below). * I considered and rejected an alternative approach in which we reduce customizability by assuming each secondary cache compatible value starts with a Slice referencing the uncompressed block contents (already true or mostly true), but we apparently intend to stack secondary caches. Saving an entry from a compressed secondary to a lower tier requires custom handling offered by SaveToCallback, etc. * Make CreateCallback part of the helper and introduce CreateContext to work with it (alternative to https://github.com/facebook/rocksdb/issues/10562). This cleans up the interface while still allowing context to be provided for loading/parsing values into primary cache. This model works for async lookup in BlockBasedTable reader (reader owns a CreateContext) under the assumption that it always waits on secondary cache operations to finish. (Otherwise, the CreateContext could be destroyed while async operation depending on it continues.) This likely contributes most to the observed performance improvement because it saves an std::function backed by a heap allocation. * Use char* for serialized data, e.g. in SaveToCallback, where void* was confusingly used. (We use `char*` for serialized byte data all over RocksDB, with many advantages over `void*`. `memcpy` etc. are legacy APIs that should not be mimicked.) * Add a type alias Cache::ObjectPtr = void*, so that we can better indicate the intent of the void* when it is to be the object associated with a Cache entry. Related: started (but did not complete) a refactoring to move away from "value" of a cache entry toward "object" or "obj". (It is confusing to call Cache a key-value store (like DB) when it is really storing arbitrary in-memory objects, not byte strings.) * Remove unnecessary key param from DeleterFn. This is good for efficiency in HyperClockCache, which does not directly store the cache key in memory. (Alternative to https://github.com/facebook/rocksdb/issues/10774) * Add allocator to Cache DeleterFn. This is a kind of future-proofing change in case we get more serious about using the Cache allocator for memory tracked by the Cache. Right now, only the uncompressed block contents are allocated using the allocator, and a pointer to that allocator is saved as part of the cached object so that the deleter can use it. (See CacheAllocationPtr.) If in the future we are able to "flatten out" our Cache objects some more, it would be good not to have to track the allocator as part of each object. * Removes legacy `ApplyToAllCacheEntries` and changes `ApplyToAllEntries` signature for Deleter->CacheItemHelper change. ## typed_cache.h Adds various "typed" interfaces to the Cache as internal APIs, so that most uses of Cache can use simple type safe code without casting and without explicit deleters, etc. Almost all of the non-test, non-glue code uses of Cache have been migrated. (Follow-up work: CompressedSecondaryCache deserves deeper attention to migrate.) This change expands RocksDB's internal usage of metaprogramming and SFINAE (https://en.cppreference.com/w/cpp/language/sfinae). The existing usages of Cache are divided up at a high level into these new interfaces. See updated existing uses of Cache for examples of how these are used. * PlaceholderCacheInterface - Used for making cache reservations, with entries that have a charge but no value. * BasicTypedCacheInterface<TValue> - Used for primary cache storage of objects of type TValue, which can be cleaned up with std::default_delete<TValue>. The role is provided by TValue::kCacheEntryRole or given in an optional template parameter. * FullTypedCacheInterface<TValue, TCreateContext> - Used for secondary cache compatible storage of objects of type TValue. In addition to BasicTypedCacheInterface constraints, we require TValue::ContentSlice() to return persistable data. This simplifies usage for the normal case of simple secondary cache compatibility (can give you a Slice to the data already in memory). In addition to TCreateContext performing the role of Cache::CreateContext, it is also expected to provide a factory function for creating TValue. * For each of these, there's a "Shared" version (e.g. FullTypedSharedCacheInterface) that holds a shared_ptr to the Cache, rather than assuming external ownership by holding only a raw `Cache*`. These interfaces introduce specific handle types for each interface instantiation, so that it's easy to see what kind of object is controlled by a handle. (Ultimately, this might not be worth the extra complexity, but it seems OK so far.) Note: I attempted to make the cache 'charge' automatically inferred from the cache object type, such as by expecting an ApproximateMemoryUsage() function, but this is not so clean because there are cases where we need to compute the charge ahead of time and don't want to re-compute it. ## block_cache.h This header is essentially the replacement for the old block_like_traits.h. It includes various things to support block cache access with typed_cache.h for block-based table. ## block_based_table_reader.cc Before this change, accessing the block cache here was an awkward mix of static polymorphism (template TBlocklike) and switch-case on a dynamic BlockType value. This change mostly unifies on static polymorphism, relying on minor hacks in block_cache.h to distinguish variants of Block. We still check BlockType in some places (especially for stats, which could be improved in follow-up work) but at least the BlockType is a static constant from the template parameter. (No more awkward partial redundancy between static and dynamic info.) This likely contributes to the overall performance improvement, but hasn't been tested in isolation. The other key source of simplification here is a more unified system of creating block cache objects: for directly populating from primary cache and for promotion from secondary cache. Both use BlockCreateContext, for context and for factory functions. ## block_based_table_builder.cc, cache_dump_load_impl.cc Before this change, warming caches was super ugly code. Both of these source files had switch statements to basically transition from the dynamic BlockType world to the static TBlocklike world. None of that mess is needed anymore as there's a new, untyped WarmInCache function that handles all the details just as promotion from SecondaryCache would. (Fixes `TODO akanksha: Dedup below code` in block_based_table_builder.cc.) ## Everything else Mostly just updating Cache users to use new typed APIs when reasonably possible, or changed Cache APIs when not. Pull Request resolved: https://github.com/facebook/rocksdb/pull/10975 Test Plan: tests updated Performance test setup similar to https://github.com/facebook/rocksdb/issues/10626 (by cache size, LRUCache when not "hyper" for HyperClockCache): 34MB 1thread base.hyper -> kops/s: 0.745 io_bytes/op: 2.52504e+06 miss_ratio: 0.140906 max_rss_mb: 76.4844 34MB 1thread new.hyper -> kops/s: 0.751 io_bytes/op: 2.5123e+06 miss_ratio: 0.140161 max_rss_mb: 79.3594 34MB 1thread base -> kops/s: 0.254 io_bytes/op: 1.36073e+07 miss_ratio: 0.918818 max_rss_mb: 45.9297 34MB 1thread new -> kops/s: 0.252 io_bytes/op: 1.36157e+07 miss_ratio: 0.918999 max_rss_mb: 44.1523 34MB 32thread base.hyper -> kops/s: 7.272 io_bytes/op: 2.88323e+06 miss_ratio: 0.162532 max_rss_mb: 516.602 34MB 32thread new.hyper -> kops/s: 7.214 io_bytes/op: 2.99046e+06 miss_ratio: 0.168818 max_rss_mb: 518.293 34MB 32thread base -> kops/s: 3.528 io_bytes/op: 1.35722e+07 miss_ratio: 0.914691 max_rss_mb: 264.926 34MB 32thread new -> kops/s: 3.604 io_bytes/op: 1.35744e+07 miss_ratio: 0.915054 max_rss_mb: 264.488 233MB 1thread base.hyper -> kops/s: 53.909 io_bytes/op: 2552.35 miss_ratio: 0.0440566 max_rss_mb: 241.984 233MB 1thread new.hyper -> kops/s: 62.792 io_bytes/op: 2549.79 miss_ratio: 0.044043 max_rss_mb: 241.922 233MB 1thread base -> kops/s: 1.197 io_bytes/op: 2.75173e+06 miss_ratio: 0.103093 max_rss_mb: 241.559 233MB 1thread new -> kops/s: 1.199 io_bytes/op: 2.73723e+06 miss_ratio: 0.10305 max_rss_mb: 240.93 233MB 32thread base.hyper -> kops/s: 1298.69 io_bytes/op: 2539.12 miss_ratio: 0.0440307 max_rss_mb: 371.418 233MB 32thread new.hyper -> kops/s: 1421.35 io_bytes/op: 2538.75 miss_ratio: 0.0440307 max_rss_mb: 347.273 233MB 32thread base -> kops/s: 9.693 io_bytes/op: 2.77304e+06 miss_ratio: 0.103745 max_rss_mb: 569.691 233MB 32thread new -> kops/s: 9.75 io_bytes/op: 2.77559e+06 miss_ratio: 0.103798 max_rss_mb: 552.82 1597MB 1thread base.hyper -> kops/s: 58.607 io_bytes/op: 1449.14 miss_ratio: 0.0249324 max_rss_mb: 1583.55 1597MB 1thread new.hyper -> kops/s: 69.6 io_bytes/op: 1434.89 miss_ratio: 0.0247167 max_rss_mb: 1584.02 1597MB 1thread base -> kops/s: 60.478 io_bytes/op: 1421.28 miss_ratio: 0.024452 max_rss_mb: 1589.45 1597MB 1thread new -> kops/s: 63.973 io_bytes/op: 1416.07 miss_ratio: 0.0243766 max_rss_mb: 1589.24 1597MB 32thread base.hyper -> kops/s: 1436.2 io_bytes/op: 1357.93 miss_ratio: 0.0235353 max_rss_mb: 1692.92 1597MB 32thread new.hyper -> kops/s: 1605.03 io_bytes/op: 1358.04 miss_ratio: 0.023538 max_rss_mb: 1702.78 1597MB 32thread base -> kops/s: 280.059 io_bytes/op: 1350.34 miss_ratio: 0.023289 max_rss_mb: 1675.36 1597MB 32thread new -> kops/s: 283.125 io_bytes/op: 1351.05 miss_ratio: 0.0232797 max_rss_mb: 1703.83 Almost uniformly improving over base revision, especially for hot paths with HyperClockCache, up to 12% higher throughput seen (1597MB, 32thread, hyper). The improvement for that is likely coming from much simplified code for providing context for secondary cache promotion (CreateCallback/CreateContext), and possibly from less branching in block_based_table_reader. And likely a small improvement from not reconstituting key for DeleterFn. Reviewed By: anand1976 Differential Revision: D42417818 Pulled By: pdillinger fbshipit-source-id: f86bfdd584dce27c028b151ba56818ad14f7a432 |
2 years ago |
|
Changyu Bi
|
0a2d3b663a |
Fix some unit test failure in ExternalSSTFileBasicTest (#11070)
Summary: valgrind build for `ExternalSSTFileBasicTest/ExternalSSTFileBasicTest.IngestFileWithMixedValueType` and `ExternalSSTFileBasicTest/ExternalSSTFileBasicTest.IngestFileWithGlobalSeqnoPickedSeqno` started failing (see error message in T141554665). I could not repro but I suspect it is due to file ingestion range overlapping with ongoing compaction, which caused a new global seqno being assigned after https://github.com/facebook/rocksdb/issues/10988. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11070 Test Plan: monitor future valgrind tests result. Reviewed By: hx235 Differential Revision: D42319056 Pulled By: cbi42 fbshipit-source-id: acbcd841a2a15e36b278f39ba514f4b9a6ee43ca |
2 years ago |
|
Niklas Fiekas
|
ff04fb154b |
Add C API for ReadOptions::async_io (#11062)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/11062 Reviewed By: hx235 Differential Revision: D42297489 Pulled By: ajkr fbshipit-source-id: 03fe1477c1ae1f8af73dc77a6986fdc7025edf4f |
2 years ago |
|
ehds
|
4737e1d41b |
fix shared state used after free (#11059)
Summary: Before this pr, the destruction order is `shared` -> `db_`(StressTest destruction) -> `stress`, but `compaction_filter` of `db_` will hold the `shared` pointer, so `shared` maybe used after free. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11059 Reviewed By: hx235 Differential Revision: D42297366 Pulled By: ajkr fbshipit-source-id: 17b314635359acacd5ba62f9db5f955f451133f7 |
2 years ago |
|
Hui Xiao
|
b965a5a80e |
Add back Options::CompactionOptionsFIFO::allow_compaction to stress/crash test (#11063)
Summary: **Context/Summary:** https://github.com/facebook/rocksdb/pull/10777 was reverted (https://github.com/facebook/rocksdb/pull/10999) due to internal blocker and replaced with a better fix https://github.com/facebook/rocksdb/pull/10922. However, the revert also reverted the `Options::CompactionOptionsFIFO::allow_compaction` stress/crash coverage added by the PR. It's an useful coverage cuz setting `Options::CompactionOptionsFIFO::allow_compaction=true` will [increase](https://github.com/facebook/rocksdb/blob/7.8.fb/db/version_set.cc#L3255) the compaction score of L0 files for FIFO and then trigger more FIFO compaction. This speed up discovery of bug related to FIFO compaction like https://github.com/facebook/rocksdb/pull/10955. To see the speedup, compare the failure occurrence in following commands with `Options::CompactionOptionsFIFO::allow_compaction=true/false` ``` --fifo_allow_compaction=1 --acquire_snapshot_one_in=10000 --adaptive_readahead=0 --allow_concurrent_memtable_write=0 --allow_data_in_errors=True --async_io=1 --avoid_flush_during_recovery=0 --avoid_unnecessary_blocking_io=1 --backup_max_size=104857600 --backup_one_in=100000 --batch_protection_bytes_per_key=0 --block_size=16384 --bloom_bits=8.869062094789008 --bottommost_compression_type=none --bytes_per_sync=0 --cache_index_and_filter_blocks=1 --cache_size=8388608 --cache_type=lru_cache --charge_compression_dictionary_building_buffer=1 --charge_file_metadata=1 --charge_filter_construction=0 --charge_table_reader=1 --checkpoint_one_in=1000000 --checksum_type=kxxHash --clear_column_family_one_in=0 --column_families=1 --compact_files_one_in=1000000 --compact_range_one_in=1000000 --compaction_pri=3 --compaction_style=2 --compaction_ttl=0 --compression_max_dict_buffer_bytes=8589934591 --compression_max_dict_bytes=16384 --compression_parallel_threads=1 --compression_type=xpress --compression_use_zstd_dict_trainer=1 --compression_zstd_max_train_bytes=0 --continuous_verification_interval=0 --data_block_index_type=0 --db=/dev/shm/rocksdb_test/rocksdb_crashtest_whitebox --db_write_buffer_size=1048576 --delpercent=4 --delrangepercent=1 --destroy_db_initially=1 --detect_filter_construct_corruption=0 --disable_wal=0 --enable_compaction_filter=0 --enable_pipelined_write=1 --expected_values_dir=/dev/shm/rocksdb_test/rocksdb_crashtest_expected --fail_if_options_file_error=1 --file_checksum_impl=xxh64 --flush_one_in=1000000 --format_version=4 --get_current_wal_file_one_in=0 --get_live_files_one_in=1000000 --get_property_one_in=1000000 --get_sorted_wal_files_one_in=0 --index_block_restart_interval=10 --index_type=2 --ingest_external_file_one_in=100 --initial_auto_readahead_size=16384 --iterpercent=10 --key_len_percent_dist=1,30,69 --level_compaction_dynamic_level_bytes=False --log2_keys_per_lock=10 --long_running_snapshots=0 --manual_wal_flush_one_in=0 --mark_for_compaction_one_file_in=10 --max_auto_readahead_size=524288 --max_background_compactions=1 --max_bytes_for_level_base=67108864 --max_key=25000000 --max_key_len=3 --max_manifest_file_size=1073741824 --max_write_batch_group_size_bytes=1048576 --max_write_buffer_number=3 --max_write_buffer_size_to_maintain=0 --memtable_prefix_bloom_size_ratio=0.01 --memtable_protection_bytes_per_key=1 --memtable_whole_key_filtering=1 --memtablerep=skip_list --min_write_buffer_number_to_merge=2 --mmap_read=0 --mock_direct_io=True --nooverwritepercent=1 --num_file_reads_for_auto_readahead=2 --open_files=-1 --open_metadata_write_fault_one_in=0 --open_read_fault_one_in=0 --open_write_fault_one_in=0 --ops_per_thread=40000 --optimize_filters_for_memory=0 --paranoid_file_checks=1 --partition_filters=0 --partition_pinning=3 --pause_background_one_in=1000000 --periodic_compaction_seconds=0 --prefix_size=7 --prefixpercent=5 --prepopulate_block_cache=0 --preserve_internal_time_seconds=3600 --progress_reports=0 --read_fault_one_in=1000 --readahead_size=0 --readpercent=15 --recycle_log_file_num=1 --reopen=0 --ribbon_starting_level=999 --secondary_cache_fault_one_in=0 --snapshot_hold_ops=100000 --sst_file_manager_bytes_per_sec=0 --sst_file_manager_bytes_per_truncate=0 --stats_dump_period_sec=0 --subcompactions=2 --sync=0 --sync_fault_injection=0 --target_file_size_base=16777216 --target_file_size_multiplier=1 --test_batches_snapshots=0 --top_level_index_pinning=1 --unpartitioned_pinning=1 --use_direct_io_for_flush_and_compaction=1 --use_direct_reads=1 --use_full_merge_v1=1 --use_merge=0 --use_multiget=0 --use_put_entity_one_in=0 --user_timestamp_size=0 --value_size_mult=32 --verify_checksum=1 --verify_checksum_one_in=1000000 --verify_db_one_in=100000 --verify_iterator_with_expected_state_one_in=5 --verify_sst_unique_id_in_manifest=1 --wal_bytes_per_sync=0 --wal_compression=none --write_buffer_size=33554432 --write_dbid_to_manifest=1 --writepercent=65 ``` Therefore this PR is adding it back to stress/crash test. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11063 Test Plan: Rehearsal stress test to make sure stress/crash test is stable Reviewed By: ajkr Differential Revision: D42283650 Pulled By: hx235 fbshipit-source-id: 132e6396ab6e24d8dcb8fe51c62dd5211cdf53ef |
2 years ago |
|
Changyu Bi
|
f24ef5d6ab |
Fix BackupEngineTest.ExcludeFiles memory leak (#11066)
Summary: Valgrind was complaining about the test BackupEngineTest.ExcludeFiles. The cause is backup_engine not being freed similar to https://github.com/facebook/rocksdb/issues/9610. ``` ==18228== Command: ./backup_engine_test --gtest_filter=BackupEngineTest.ExcludeFiles ==18228== Note: Google Test filter = BackupEngineTest.ExcludeFiles [==========] Running 1 test from 1 test case. [----------] Global test environment set-up. [----------] 1 test from BackupEngineTest [ RUN ] BackupEngineTest.ExcludeFiles [ OK ] BackupEngineTest.ExcludeFiles (16264 ms) [----------] 1 test from BackupEngineTest (16273 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test case ran. (16306 ms total) [ PASSED ] 1 test. ==18228== ==18228== HEAP SUMMARY: ==18228== in use at exit: 14,099 bytes in 159 blocks ==18228== total heap usage: 255,328 allocs, 255,169 frees, 497,538,546 bytes allocated ==18228== ==18228== 19 bytes in 1 blocks are possibly lost in loss record 4 of 67 ==18228== at 0x483BE63: operator new(unsigned long) (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==18228== by 0x1E752D: void std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char*>(char*, char*, std::forward_iterator_tag) [clone .constprop.0] (basic_string.tcc:219) ==18228== by 0x1F1898: _M_construct_aux<char*> (basic_string.h:251) ==18228== by 0x1F1898: _M_construct<char*> (basic_string.h:270) ==18228== by 0x1F1898: basic_string (basic_string.h:455) ==18228== by 0x1F1898: construct<std::__cxx11::basic_string<char>, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&> (new_allocator.h:146) ==18228== by 0x1F1898: construct<std::__cxx11::basic_string<char>, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&> (alloc_traits.h:483) ==18228== by 0x1F1898: push_back (stl_vector.h:1189) ==18228== by 0x1F1898: rocksdb::(anonymous namespace)::TestFs::NewWritableFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, rocksdb::FileOptions const&, std::unique_ptr<rocksdb::FSWritableFile, std::default_delete<rocksdb::FSWritableFile> >*, rocksdb::IODebugContext*) (backup_engine_test.cc:208) ==18228== by 0x4B3583: rocksdb::NewWritableFile(rocksdb::FileSystem*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::unique_ptr<rocksdb::FSWritableFile, std::default_delete<rocksdb::FSWritableFile> >*, rocksdb::FileOptions const&) (read_write_util.cc:23) ==18228== by 0x31C3A8: rocksdb::DBImpl::CreateWAL(unsigned long, unsigned long, unsigned long, rocksdb::log::Writer**) (db_impl_open.cc:1752) ==18228== by 0x321A8C: rocksdb::DBImpl::Open(rocksdb::DBOptions const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<rocksdb::ColumnFamilyDescriptor, std::allocator<rocksdb::ColumnFamilyDescriptor> > const&, std::vector<rocksdb::ColumnFamilyHandle*, std::allocator<rocksdb::ColumnFamilyHandle*> >*, rocksdb::DB**, bool, bool) (db_impl_open.cc:1852) ==18228== by 0x322E7F: Open (db_impl_open.cc:1660) ==18228== by 0x322E7F: rocksdb::DB::Open(rocksdb::Options const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, rocksdb::DB**) (db_impl_open.cc:1637) ==18228== by 0x1EE1CD: InitializeDBAndBackupEngine (backup_engine_test.cc:724) ==18228== by 0x1EE1CD: rocksdb::(anonymous namespace)::BackupEngineTest::OpenDBAndBackupEngine(bool, bool, rocksdb::(anonymous namespace)::BackupEngineTest::ShareOption) (backup_engine_test.cc:732) ==18228== by 0x217585: rocksdb::(anonymous namespace)::BackupEngineTest_ExcludeFiles_Test::TestBody() (backup_engine_test.cc:4232) ==18228== by 0x296143: HandleSehExceptionsInMethodIfSupported<testing::Test, void> (gtest-all.cc:3899) ==18228== by 0x296143: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) (gtest-all.cc:3935) ==18228== by 0x28A0A5: testing::Test::Run() [clone .part.0] (gtest-all.cc:3973) ==18228== by 0x28A364: Run (gtest-all.cc:3965) ==18228== by 0x28A364: testing::TestInfo::Run() [clone .part.0] (gtest-all.cc:4149) ... ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/11066 Test Plan: make -j24 J=24 ROCKSDBTESTS_SUBSET=backup_engine_test valgrind_check_some Reviewed By: ajkr Differential Revision: D42297791 Pulled By: cbi42 fbshipit-source-id: db67982b27b91cc78e1a9f4a96da0cba7c9785b7 |
2 years ago |
|
mrambacher
|
559aaa3577 |
Add ability to have unit tests for ROCKSDB_PLUGINS (#11052)
Summary: This is based on speedb PR [143](https://github.com/speedb-io/speedb/pull/143). This PR adds the ability to add a xxx_TESTS variable to the make or cmake files for a plugin. When set, those files will be added to the unit tests built and executed by the corresponding make system. Note that the rule for building plugin tests via make could be expanded to almost every other unit test in RocksDB. This expansion would allow for a much smaller/simpler Makefile and make it easier to add new test files to RocksDB. Pull Request resolved: https://github.com/facebook/rocksdb/pull/11052 Reviewed By: cbi42 Differential Revision: D42212269 Pulled By: ajkr fbshipit-source-id: d02668f7f4466900d63c90bb4f7962d23fcc7114 |
2 years ago |
|
ywave
|
7f71880de9 |
Fix typo in flushing stats CF (#11055)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/11055 Test Plan: make check Reviewed By: cbi42 Differential Revision: D42232828 Pulled By: ajkr fbshipit-source-id: 3b46514aebff4da7e47b9954b90800ba4a3ba30b |
2 years ago |
|
HuangYi
|
33aca893c2 |
add c-api for setting option optimize_filters_for_memory (#11044)
Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/11044 Reviewed By: cbi42 Differential Revision: D42152851 Pulled By: ajkr fbshipit-source-id: 81710d9503ba4f23f112c72ebf16a48112e27158 |
2 years ago |
|
Hui Xiao
|
9502856edd |
Add missing range conflict check between file ingestion and RefitLevel() (#10988)
Summary: **Context:** File ingestion never checks whether the key range it acts on overlaps with an ongoing RefitLevel() (used in `CompactRange()` with `change_level=true`). That's because RefitLevel() doesn't register and make its key range known to file ingestion. Though it checks overlapping with other compactions by https://github.com/facebook/rocksdb/blob/7.8.fb/db/external_sst_file_ingestion_job.cc#L998. RefitLevel() (used in `CompactRange()` with `change_level=true`) doesn't check whether the key range it acts on overlaps with an ongoing file ingestion. That's because file ingestion does not register and make its key range known to other compactions. - Note that non-refitlevel-compaction (e.g, manual compaction w/o RefitLevel() or general compaction) also does not check key range overlap with ongoing file ingestion for the same reason. - But it's fine. Credited to cbi42's discovery, `WaitForIngestFile` was called by background and foreground compactions. They were introduced in |
2 years ago |
