Bringing the latest commits form the master repo #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: Seems it's causing some tests failures. Pull Request resolved: facebook#7350 Reviewed By: riversand963 Differential Revision: D23544109 Pulled By: jay-zhuang fbshipit-source-id: 798a0ca374a20b6c2d0f29582729ff101c6a2e99
Summary: Pull Request resolved: facebook#7315 Test Plan: `ASSERT_STATUS_CHECKED=1 make sst_dump_test && ./sst_dump_test` And manually run `./sst_dump --file=*.sst` before and after the change. Reviewed By: pdillinger Differential Revision: D23361669 Pulled By: jay-zhuang fbshipit-source-id: 5bf51a2a90ee35c8c679e5f604732ec2aef5949a
Summary: Currently, application may pass a statistics object to db but later wants to reduce stats tracking overhead by setting stats level to kExceptHistogramOrTimers (the current lowest level). Tickers will still be incremented, causing up to 1% CPU. We can add a new lowest stats level `kExceptTickers` to disable ticker incrementing as well, thus reducing CPU cycles spent on tickers. Test Plan (devserver): ``` make check make clean DEBUG_LEVEL=0 make db_bench ./db_bench -perf_level=1 -stats_level=0 -statistics -benchmarks=fillseq,readrandom -duration=120 ``` Measure CPU util (%) before and after change: CPU util by rocksdb::RecordTick: 1.1 vs (<0.1) Pull Request resolved: facebook#7329 Reviewed By: pdillinger Differential Revision: D23434014 Pulled By: riversand963 fbshipit-source-id: 72ff0f02a192ac476d4b0044b9f37fd4a22ff0d4
Summary: The patch cleans up a few things in `CompactionJob::SubcompactionState`: * Instead of using both the member initializer list and in-class initializers (and sometimes both at the same time for the same member), the struct now uniformly uses the latter to initialize integer members. * The default parameter value for the constructor parameter `size` is removed. * The explicitly deleted copy operations are removed, since they are implicitly deleted anyways because of the `unique_ptr` members. * The handwritten move operations, which did not move the member `c_iter` and were not declared `nothrow`, are removed. Note that with the user-declared copy operations gone (see the previous item), we can rely on the compiler to (correctly) generate these methods. Pull Request resolved: facebook#7322 Test Plan: `make check` Reviewed By: siying Differential Revision: D23382408 Pulled By: ltamasi fbshipit-source-id: a4ae5af150161c50ff7bdc07fa145482d0150bfe
Summary: (1) Skip check on specific key if restoring an old backup (small minority of cases) because it can fail in those cases. (2) Remove an old assertion about number of column families and number of keys passed in, which is broken by atomic flush (cf_consistency) test. Like other code (for better or worse) assume a single key and iterate over column families. (3) Apply mock_direct_io to NewSequentialFile so that db_stress backup works on /dev/shm. Also add more context to output in case of backup/restore db_stress failure. Also a minor fix to BackupEngine to report first failure status in creating new backup, and drop another clue about the potential source of a "Backup failed" status. Reverts "Disable backup/restore stress test (facebook#7350)" Pull Request resolved: facebook#7357 Test Plan: Using backup_one_in=10000, "USE_CLANG=1 make crash_test_with_atomic_flush" for 30+ minutes "USE_CLANG=1 make blackbox_crash_test" for 30+ minutes And with use_direct_reads with TEST_TMPDIR=/dev/shm/rocksdb Reviewed By: riversand963 Differential Revision: D23567244 Pulled By: pdillinger fbshipit-source-id: e77171c2e8394d173917e36898c02dead1c40b77
Summary:
Replace FSWritableFile pointer with FSWritableFilePtr
object in WritableFileWriter.
This new object wraps FSWritableFile pointer.
Objective: If tracing is enabled, FSWritableFile Ptr returns
FSWritableFileTracingWrapper pointer that includes all necessary
information in IORecord and calls underlying FileSystem and invokes
IOTracer to dump that record in a binary file. If tracing is disabled
then, underlying FileSystem pointer is returned directly.
FSWritableFilePtr wrapper class is added to bypass the
FSWritableFileWrapper when
tracing is disabled.
Test Plan: make check -j64
Pull Request resolved: facebook#7193
Reviewed By: anand1976
Differential Revision: D23355915
Pulled By: akankshamahajan15
fbshipit-source-id: e62a27a13c1fd77e36a6dbafc7006d969bed25cf
Summary: As title. Pull Request resolved: facebook#7355 Reviewed By: pdillinger Differential Revision: D23566635 Pulled By: riversand963 fbshipit-source-id: f8d846bcff637e7617b764b7bfb9a948ea18d195
Summary: gcc-4.8 returns error when using the constructor. Not sure if it's a compiler bug/limitation or code issue: ``` table/block_based/block_based_table_reader.cc:3183:67: error: use of deleted function ‘rocksdb::WritableFileStringStreamAdapter::WritableFileStringStreamAdapter(rocksdb::WritableFileStringStreamAdapter&&)’ ``` Pull Request resolved: facebook#7358 Reviewed By: pdillinger Differential Revision: D23577651 Pulled By: jay-zhuang fbshipit-source-id: b0197e3d3538da61a6f3866410d88d2047fb9695
…k#7198) Summary: Replace FSRandomRWFile pointer with FSRandomRWFilePtr object in the rocksdb internal code. This new object wraps FSRandomRWFile pointer. Objective: If tracing is enabled, FSRandomRWFile object returns FSRandomRWFileTracingWrapper pointer that includes all necessary information in IORecord and calls underlying FileSystem and invokes IOTracer to dump that record in a binary file. If tracing is disabled then, underlying FileSystem pointer is returned directly. FSRandomRWFilePtr wrapper class is added to bypass the FSRandomRWFileWrapper when tracing is disabled. Pull Request resolved: facebook#7198 Test Plan: make check -j64 Reviewed By: anand1976 Differential Revision: D23421116 Pulled By: akankshamahajan15 fbshipit-source-id: 8a5ba0e7d9c1ba34c3a6f29829b107c5f09ab6a3
Summary:
Also enables a pull request to trigger all the Travis
configurations by writing FULL_CI in the commit message. (See what I did
there?)
First issue
make: *** No rule to make target 'jl/util/crc32c_ppc_asm.o', needed by 'rocksdbjava'. Stop.
Second issue
tools/db_bench_tool.cc:5514:38: error: ‘gen_exp.rocksdb::Benchmark::GenerateTwoTermExpKeys::keyrange_size_’ may be used uninitialized in this function
Pull Request resolved: facebook#7359
Test Plan: CI
Reviewed By: zhichao-cao
Differential Revision: D23582132
Pulled By: pdillinger
fbshipit-source-id: 06d794673fd522ba11cf6398385387e6bd97ef89
Summary: To fix the cmake build with third_party libs, like: `mkdir build && cd build && cmake .. -DWITH_SNAPPY=1 && make` Error: ``` Undefined symbols for architecture x86_64: "snappy::RawCompress(char const*, unsigned long, char*, unsigned long*)" ``` Pull Request resolved: facebook#7351 Reviewed By: pdillinger Differential Revision: D23553705 Pulled By: jay-zhuang fbshipit-source-id: 19b45c6763c7256107583e8af4c01d370ca06128
Summary: 1. Failed to compile because of use of FileSystem* instead of Env* to some methods; 2. Failed to compile with addition of ConfigOptions to some methods 3. Failed to run successfully because the database and/or db_bench would change some of the options, invalidating the comparison 4. Failed to run successfully if Snappy was not available. Pull Request resolved: facebook#7344 Reviewed By: siying Differential Revision: D23501093 Pulled By: jay-zhuang fbshipit-source-id: 81fd947e95fff9db8a4c5ff419d69d4c36bef23f
Summary: Was broken by facebook#6660 Travis times before this change, after 6660: platform_dependent: 17 min group 1: 15 min group 2: 44 min (often timeout on non-x86 or non-Linux) group 3: 31 min group 4: 21 min After this change: TODO Pull Request resolved: facebook#7360 Test Plan: CI inspection Reviewed By: ajkr Differential Revision: D23586917 Pulled By: pdillinger fbshipit-source-id: 4c67cf33180b0b833c39a817e6c1f128727941d2
…s added in VersionBuilder (facebook#7349) Summary: When multiple background jobs are generating blob files in parallel, it is actually possible for a blob file to be added with a file number that is lower than the highest one in the base version. (This is a harmless race condition.) The patch fixes the handling of this case and adds a unit test. Pull Request resolved: facebook#7349 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D23542453 Pulled By: ltamasi fbshipit-source-id: 4ff6f3654bc58c391d10b9870e1cc40b5e3fa8e4
Summary: (a) Missed a case in updating handling of rand_keys (b) Only opening restored db with DB::Open so don't (yet) attempt to open restored BlobDB or TransactionDB. Pull Request resolved: facebook#7361 Test Plan: better than being broken Reviewed By: ajkr Differential Revision: D23592570 Pulled By: pdillinger fbshipit-source-id: dd1d999bcc0c852ee77cb6041964ec4abc0fd4fd
…acebook#7321) Summary: Pull Request resolved: facebook#7321 Reviewed By: ajkr Differential Revision: D23590160 fbshipit-source-id: 35d106e732ac37f674222759cdb1dbb31e005ca7
…ead_limiter field in ColumnFamilyOptions (facebook#7347) Summary: as title Pull Request resolved: facebook#7347 Test Plan: unit tests included Reviewed By: jay-zhuang Differential Revision: D23592552 Pulled By: pdillinger fbshipit-source-id: 1c3571b6f42bfd0cfd723ff49d01fbc02a1be45b
…iveFileMetaData (facebook#7365) Summary: The patch adds support for exposing the start of the expiration range for TTL blob files through the `GetLiveFilesMetaData` API. This can be used for monitoring purposes, i.e. to make sure TTL blob files are deleted in a timely manner. The patch also fixes a couple of uninitialized variable issues. Pull Request resolved: facebook#7365 Test Plan: `make check` Reviewed By: pdillinger Differential Revision: D23605465 Pulled By: ltamasi fbshipit-source-id: 97a9612bf5f4b058423debdd3f28f576bb23a70f
…acebook#7369) Summary: facebook#3341 guaranteed that upon return of `GetSortedWalFiles` after `DisableFileDeletions`, all pending purges of previously obsolete WAL files will have finished. However, the addition of avoid_unnecessary_blocking_io in facebook#5043 opened a hole in the code making that assurance, which can lead to files to be copied for checkpoint or backup going missing before being copied, with that option enabled. This change patches the hole. Pull Request resolved: facebook#7369 Test Plan: apparent fix to backups in crash test observed. Will work on a unit test for another commit Reviewed By: ajkr Differential Revision: D23620258 Pulled By: pdillinger fbshipit-source-id: bea36b461a5b719c3e3ef802f967bc3e8ae71614
Summary: We can only check key on restored backup if in a stress test configuration locking the key. (Fixes mismatch seen in backup/restore with atomic flush.) TestCheckpoint used a very ugly solution to the same problem: copy-paste dozens of lines of code with some changes and removals. I removed the unnecessary implementation and made the existing one simply adaptive, like TestBackupRestore. Also made TestBackupRestore clean up dead backup/restore directories on success. Pull Request resolved: facebook#7373 Test Plan: blackbox_crash_test_with_atomic_flush for a while, blackbox_crash_test for a while, with backup and checkpoint 1 in 5k and only 1k max_keys to stress this area Reviewed By: ajkr Differential Revision: D23629057 Pulled By: pdillinger fbshipit-source-id: d7fe7e2be75aaf3cf974be9540a7c5c5de8b371b
Summary: We've seen some segfaults in db_write_test, with at least one suggesting corruption of a write group linked list. Adding an assertion to have this fail in a more specific way if that is the broken invariant. Pull Request resolved: facebook#7375 Test Plan: make check Reviewed By: jay-zhuang Differential Revision: D23638477 Pulled By: pdillinger fbshipit-source-id: a76fd677cad60a3a516bd363947bfd9ce418edc1
Summary: During bottommost compaction, RocksDB cannot simply drop a tombstone if this tombstone is not in the earliest snapshot. The current behavior is: RocksDB skips other internal keys (of the same user key) in the same snapshot range. In the meantime, RocksDB should check for the `shutting_down` flag. Otherwise, it is possible for a bottommost compaction that has already started running to take a long time to finish, even if the application has tried to cancel all background jobs. Pull Request resolved: facebook#7356 Test Plan: make check Reviewed By: ltamasi Differential Revision: D23663241 Pulled By: riversand963 fbshipit-source-id: 25f8e9b51bc3bfa3353cdf87557800f9d90ee0b5
Summary: for release fork Pull Request resolved: facebook#7378 Test Plan: make check + CI Reviewed By: jay-zhuang Differential Revision: D23669163 Pulled By: pdillinger fbshipit-source-id: 14cbf95b32717c28418c71cc8e10f06733bbc49f
Summary: Covered methods: - OldDefaults() - OptimizeForSmallDb(std::shared_ptr<Cache>) Covered fields: - cf_paths Pull Request resolved: facebook#7372 Reviewed By: pdillinger Differential Revision: D23683449 Pulled By: jay-zhuang fbshipit-source-id: 3e5a8b657cc382c19de3a48c666a3b0e8d96968d
) Summary: This is potentially the cause of failures: Failure in Destroy restore dir with: IO error: file rmdir: /dev/shm/rocksdb/rocksdb_crashtest_whitebox/.restore13: Directory not empty Pull Request resolved: facebook#7384 Test Plan: smoke test blackbox_crash_test Reviewed By: jay-zhuang Differential Revision: D23685087 Pulled By: pdillinger fbshipit-source-id: 55f62e9853ce84be1d5ca7d856de867f0f2596ee
Summary: snapd update has been failing on ppc for ~a week. Disabling it for now in pull requests. Also, facebook#6653 seems to be fixed, so re-enabling standard unit tests for PPC on pull requests. Pull Request resolved: facebook#7381 Test Plan: CI Reviewed By: jay-zhuang Differential Revision: D23684962 Pulled By: pdillinger fbshipit-source-id: 96ec9487b714c4741bb1653dae90b24118830cb5
…cebook#7374) Summary: In a distributed file system, directory ownership is enforced by fencing off the previous owner once they've been preempted by a new owner. This PR adds a IOStatus subcode for ```StatusCode::IOError``` to indicate this. Once this error is returned for a file write, the DB is put in read-only mode and not allowed to resume in read-write mode. Pull Request resolved: facebook#7374 Test Plan: Add new unit tests in ```error_handler_fs_test``` Reviewed By: riversand963 Differential Revision: D23687777 Pulled By: anand1976 fbshipit-source-id: bef948642089dc0af399057864d9a8ca339e8b2f
Summary: This PR merges the functionality of making the ColumnFamilyOptions, TableFactory, and DBOptions into Configurable into a single PR, resolving any merge conflicts Pull Request resolved: facebook#5753 Reviewed By: ajkr Differential Revision: D23385030 Pulled By: zhichao-cao fbshipit-source-id: 8b977a7731556230b9b8c5a081b98e49ee4f160a
Summary: Introduced `valgrind_check_some`, which is analogous to the `check_some` target for non-valgrind tests. It simplifies the process for running a single valgrind test or subset of valgrind tests when trying to repro a failure. I also added a `ROCKSDBTESTS_ONLY` parameter, which simplifies selecting a single test to run. Previously the user would have to use `ROCKSDBTESTS_START` and `ROCKSDBTESTS_END`, but it was difficult to determine the end variable since it is an exclusive endpoint and must match an actual test name. Pull Request resolved: facebook#7379 Reviewed By: pdillinger Differential Revision: D23673608 Pulled By: ajkr fbshipit-source-id: 87ed81f1a671d46c2dff6a701f85f1891c725b3f
Summary: The patch adds support for writing blob files during flush by integrating `BlobFileBuilder` with the flush logic, most importantly, `BuildTable` and `CompactionIterator`. If `enable_blob_files` is set, large values are extracted to blob files and replaced with references. The resulting blob files are then logged to the MANIFEST as part of the flush job's `VersionEdit` and added to the `Version`, similarly to table files. Errors related to writing blob files fail the flush, and any blob files written by such jobs are immediately deleted (again, similarly to how SST files are handled). In addition, the patch extends the logging and statistics around flushes to account for the presence of blob files (e.g. `InternalStats::CompactionStats::bytes_written`, which is used for calculating write amplification, now considers the blob files as well). Pull Request resolved: facebook#7345 Test Plan: Tested using `make check` and `db_bench`. Reviewed By: riversand963 Differential Revision: D23506369 Pulled By: ltamasi fbshipit-source-id: 646885f22dfbe063f650d38a1fedc132f499a159
Summary: Pull Request resolved: facebook#7518 Test Plan: ``` $USE_CLANG=1 make analyze ``` Reviewed By: zhichao-cao Differential Revision: D24175390 Pulled By: riversand963 fbshipit-source-id: c70121652908cf5d450120c38ab65cc595332ca7
Summary: Add ldb_cmd_test to ASSERT_STATUS_CHECKED list Pull Request resolved: facebook#7499 Test Plan: pass ASSERT_STATUS_CHECKED=1 make -j48 ldb_cmd_test Reviewed By: cheng-chang Differential Revision: D24086203 Pulled By: zhichao-cao fbshipit-source-id: 29592202b1d4335e566de15e7937269d98d57841
Summary: It's a known issue, which is tracked in facebook#7405, facebook#7470. Disable it for now. Pull Request resolved: facebook#7511 Reviewed By: zhichao-cao Differential Revision: D24145075 Pulled By: jay-zhuang fbshipit-source-id: 1858497972f2baba617867aaeac30d93b8305c80
…facebook#7519) Summary: The `std::pair(const T1& x, const T2& y);` constructor is `constexpr` only starting from C++14; relying on this breaks compilation on certain compilers/platforms we need to support. Pull Request resolved: facebook#7519 Test Plan: `make check` Reviewed By: ajkr Differential Revision: D24195747 Pulled By: ltamasi fbshipit-source-id: 665e8fbc9747675bb49c5d895aad3dcf2714750f
Summary: Add status enforcement for following tests: 1. import_column_family_test 2. memory_test 3. table_test Pull Request resolved: facebook#7500 Reviewed By: zhichao-cao Differential Revision: D24095887 Pulled By: akankshamahajan15 fbshipit-source-id: db8e1ec595852df143fad78a0c07bfdd27dc3c84
Summary: Cover paranoid_file_checks in crash test. Pull Request resolved: facebook#7489 Test Plan: Run crash tests for hours and didn't see any failure. Reviewed By: ajkr Differential Revision: D24063868 fbshipit-source-id: 7b48b110e66ce78ae5d0c99a9f32af86edd34c1e
Summary: Update release history for 6.14 Pull Request resolved: facebook#7525 Test Plan: No code change Reviewed By: jay-zhuang Differential Revision: D24224690 Pulled By: akankshamahajan15 fbshipit-source-id: 95441aefde96672fea5a6af5d7e67cdafb1ebdd2
…acebook#7275) Summary: This option determines whether WALs will be tracked in MANIFEST and verified on recovery. Pull Request resolved: facebook#7275 Test Plan: db_options_test options_test Reviewed By: pdillinger Differential Revision: D23181418 Pulled By: cheng-chang fbshipit-source-id: 5dd1cdc166f3dfc1c93c094df4a2f7734e3b4547
Summary: The old flag-based APIs (`BlockBasedTableOptions::pin_l0_filter_and_index_blocks_in_cache` and `BlockBasedTableOptions::pin_top_level_index_and_filter`) were insufficient for our needs. For example, it was impossible to pin only unpartitioned meta-blocks, which could prevent block cache contention when turning on dictionary compression or during a migration to partitioned indexes/filters. It was also impossible to pin all meta-blocks in memory while having predictable memory usage via block cache. If we had continued adding flags to address these scenarios, they would have had significant overlap causing confusion. Instead, this PR deprecates the flags and starts a new API with non-overlapping options. Pull Request resolved: facebook#7520 Test Plan: - new unit test - added new options to stress/crash test and ran for a while: `$ python tools/db_crashtest.py blackbox --simple --max_key=1000000 -write_buffer_size=1048576 -target_file_size_base=1048576 -max_bytes_for_level_base=4194304 --interval=10 -value_size_mult=33 -column_families=1 -reopen=0` Reviewed By: pdillinger Differential Revision: D24200034 Pulled By: ajkr fbshipit-source-id: 3fa7cfc71e7960f7a867511dd6ae5834dd73b13e
Summary: as title Pull Request resolved: facebook#7536 Test Plan: see the new `build-macos` tests pass in circleci Reviewed By: jay-zhuang Differential Revision: D24243218 Pulled By: cheng-chang fbshipit-source-id: 9b5f8a859e54c99a9ebe7efff6f336458a5d42de
Summary: ThreadSanitizer: data race for `DummyOldStats.num_rt`. Failed build: https://app.circleci.com/pipelines/github/facebook/rocksdb/3991/workflows/b47c3ae1-5531-4489-ac51-11854abdfd0f/jobs/42305 Pull Request resolved: facebook#7526 Reviewed By: akankshamahajan15 Differential Revision: D24226736 Pulled By: jay-zhuang fbshipit-source-id: e05ce354d0c0db0eba242d59d4b0e89ce7c25acf
…acebook#7534) Summary: If crash test fails, don't delete the `expected_values_file` for later debug. More details: facebook#7530 Pull Request resolved: facebook#7534 Test Plan: local host Reviewed By: ajkr Differential Revision: D24239655 Pulled By: jay-zhuang fbshipit-source-id: 3566f91a30aae1e27d2f51d910cddd08edb7d4cf
Summary: Added status check enforcement for db_flush_test Pull Request resolved: facebook#7476 Test Plan: ASSERT_STATUS_CHECKED=1 make -j48 db_flush_test Reviewed By: akankshamahajan15 Differential Revision: D24033752 Pulled By: zhichao-cao fbshipit-source-id: d957934e1666d0043bebdd8a4149e94cdcbbb89b
Summary: Added unit tests that have paranoid_check = true that perform range deletions. At the moment, the deleted ranges do not appear to be checked as part of the paranoid checks. Pull Request resolved: facebook#7521 Reviewed By: zhichao-cao Differential Revision: D24262175 Pulled By: ajkr fbshipit-source-id: 1035e968f7ab8ccaa7af086b835a4e72c7e56743
Summary: Add plain_table_db_test to ASSERT_STATUS_CHECKED list Pull Request resolved: facebook#7482 Test Plan: ASSERT_STATUS_CHECKED=1 make -j48 plain_table_db_test Reviewed By: riversand963 Differential Revision: D24034987 Pulled By: zhichao-cao fbshipit-source-id: e61c937d55ded0947cc8936937362dafed572a60
Summary: These notes existed on the release branches where they were backported, but were never added on master branch. Added them now and mentioned what minor release the fix originally appeared. Pull Request resolved: facebook#7545 Reviewed By: riversand963 Differential Revision: D24281759 Pulled By: ajkr fbshipit-source-id: 7422e984b667793d6260dd32a7492afcb2ff1c4b
Summary: db_stress prints key in both id and hex. facebook#7531 Pull Request resolved: facebook#7533 Test Plan: local test Reviewed By: akankshamahajan15 Differential Revision: D24252725 Pulled By: jay-zhuang fbshipit-source-id: f0c1409a0568874df36949d5da139316d978fa98
Summary: Pull Request resolved: facebook#7549 Reviewed By: ajkr, zhichao-cao Differential Revision: D24292032 Pulled By: jay-zhuang fbshipit-source-id: 0442283386ae20d10410a8d013a431d7cd282b22
Summary: Update IOTrace operations in stackabledb.h and also trace few other IO operations. Pull Request resolved: facebook#7514 Test Plan: make check -j64 Reviewed By: anand1976 Differential Revision: D24151202 Pulled By: akankshamahajan15 fbshipit-source-id: 112cd3d2041f8c6398b7b0ba1a783b8c93224d4a
…ook#7543) Summary: Right now all I/O failures under PartitionFilterBlock::CacheDependencies() is swallowed. Return error in case prefetch fails. On returning error in PartitionedFilterBlockReader::CacheDependencies was causing stress test failure because PrefetchBuffer is initialized with enable_ = true, as result when PosixMmapReadableFile::Read is called from Prefetch, scratch is ignored causing buffer to fill with garbage values. Initializing prefetch buffer by CreatePrefetchBuffer that sets enable_ with !ioptions.allow_mmap_reads fixed the problem as it returns without prefetching data if allow_mmap_reads is set. Pull Request resolved: facebook#7543 Test Plan: make check -j64; python -u tools/db_crashtest.py --simple blackbox Reviewed By: anand1976 Differential Revision: D24284596 Pulled By: akankshamahajan15 fbshipit-source-id: f3f0fd44b59dcf60645730436f28564a07884868
…acebook#7555) Summary: Pull Request resolved: facebook#7555 Test Plan: ASSERT_STATUS_CHECKED=1 make -j48 error_handler_fs_test Reviewed By: riversand963 Differential Revision: D24299387 Pulled By: zhichao-cao fbshipit-source-id: 6c8aa91c4b6e2bc82580b8d2264c177068f5a32c
Summary: Allows adding event listeners in RocksJava. * Adds listeners getter and setter in `Options` and `DBOptions` classes. * Adds `EventListener` Java interface and base class for implementing custom event listener callbacks - `AbstractEventListener`, which has an underlying native callback class implementing C++ `EventListener` class. * `AbstractEventListener` class has mechanism for selectively enabling its callback methods in order to prevent invoking Java method if it is not implemented. This decreases performance cost in case only subset of event listener callback methods is needed - the JNI code for remaining "no-op" callbacks is not executed. * The code is covered by unit tests in `EventListenerTest.java`, there are also tests added for setting/getting listeners field in `OptionsTest.java` and `DBOptionsTest.java`. Pull Request resolved: facebook#7425 Reviewed By: pdillinger Differential Revision: D24063390 Pulled By: jay-zhuang fbshipit-source-id: 508c359538983d6b765e70d9989c351794a944ee
Summary: fix for clang_analyzer build failure in table_test because of potential memory leak of memtable in case of ASSERT failure. Pull Request resolved: facebook#7553 Test Plan: USE_CLANG=1 make analyze; make check -j64 Reviewed By: jay-zhuang Differential Revision: D24295042 Pulled By: akankshamahajan15 fbshipit-source-id: e9ea184367970fff3b520e33f3ceebf28d66ac8d
…Handle instances (facebook#7428) Summary: - Takes the burden off developer to close ColumnFamilyHandle instances before closing RocksDB instance - The change is backward-compatible ---- Previously the pattern for working with Column Families was: ```java try (final ColumnFamilyOptions cfOpts = new ColumnFamilyOptions().optimizeUniversalStyleCompaction()) { // list of column family descriptors, first entry must always be default column family final List<ColumnFamilyDescriptor> cfDescriptors = Arrays.asList( new ColumnFamilyDescriptor(RocksDB.DEFAULT_COLUMN_FAMILY, cfOpts), new ColumnFamilyDescriptor("my-first-columnfamily".getBytes(), cfOpts) ); // a list which will hold the handles for the column families once the db is opened final List<ColumnFamilyHandle> columnFamilyHandleList = new ArrayList<>(); try (final DBOptions options = new DBOptions() .setCreateIfMissing(true) .setCreateMissingColumnFamilies(true); final RocksDB db = RocksDB.open(options, "path/to/do", cfDescriptors, columnFamilyHandleList)) { try { // do something } finally { // NOTE user must explicitly frees the column family handles before freeing the db for (final ColumnFamilyHandle columnFamilyHandle : columnFamilyHandleList) { columnFamilyHandle.close(); } } // frees the column family options } } // frees the db and the db options ``` With the changes in this PR, the Java user no longer has to worry about manually closing the Column Families, which allows them to write simpler symmetrical create/free oriented code like this: ```java try (final ColumnFamilyOptions cfOpts = new ColumnFamilyOptions().optimizeUniversalStyleCompaction()) { // list of column family descriptors, first entry must always be default column family final List<ColumnFamilyDescriptor> cfDescriptors = Arrays.asList( new ColumnFamilyDescriptor(RocksDB.DEFAULT_COLUMN_FAMILY, cfOpts), new ColumnFamilyDescriptor("my-first-columnfamily".getBytes(), cfOpts) ); // a list which will hold the handles for the column families once the db is opened final List<ColumnFamilyHandle> columnFamilyHandleList = new ArrayList<>(); try (final DBOptions options = new DBOptions() .setCreateIfMissing(true) .setCreateMissingColumnFamilies(true); final RocksDB db = RocksDB.open(options, "path/to/do", cfDescriptors, columnFamilyHandleList)) { // do something } // frees the column family options, then frees the db and the db options } } ``` **NOTE**: The changes in this PR are backwards API compatible, which means existing code using the original approach will also continue to function correctly. Pull Request resolved: facebook#7428 Reviewed By: cheng-chang Differential Revision: D24063348 Pulled By: jay-zhuang fbshipit-source-id: 648d7526669923128c863ead94516bf4d50ac658
Summary: Pull Request resolved: facebook#7550 Reviewed By: jay-zhuang Differential Revision: D24315343 Pulled By: ajkr fbshipit-source-id: fc7855b630a50c00dcb940241942295932732f39
Summary: * Avoid some unnecessary array copy operations on read/write * Remove some duplicated code * Don't leak arrays on some exceptions * Fixed some doc comments Pull Request resolved: facebook#7371 Reviewed By: jay-zhuang Differential Revision: D24312932 Pulled By: pdillinger fbshipit-source-id: 422fe6b98bbdb922a148922ac0d2d965c715176e
Summary: Make LoadLatestOptions return PathNotFound if the options file does not exist. Added tests for the LoadOptions related methods. Pull Request resolved: facebook#7554 Reviewed By: akankshamahajan15 Differential Revision: D24298985 Pulled By: zhichao-cao fbshipit-source-id: c9ae3cb12fc4a5bbef07743e1c1300f98a2441b3
Summary: Pull Request resolved: facebook#7560 Reviewed By: akankshamahajan15 Differential Revision: D24323048 Pulled By: zhichao-cao fbshipit-source-id: 2094ef089c37f570d4ea30d28d2f46f9fa1ce0f1
merge master
sgladkov
pushed a commit
that referenced
this pull request
May 16, 2022
…n opening Posix WritableFile (facebook#9685) Summary: Pull Request resolved: facebook#9685 Our TSAN reports a race condition as follows when running test ``` gtest-parallel -r 100 ./external_sst_file_test --gtest_filter=ExternalSSTFileTest.MultiThreaded ``` leads to the following ``` WARNING: ThreadSanitizer: data race (pid=2683148) Write of size 1 at 0x556fede63340 by thread T7: #0 rocksdb::(anonymous namespace)::PosixFileSystem::OpenWritableFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, rocksdb::FileOptions const&, bool, std::unique_ptr<rocksdb::FSWritableFile, std::default_delete<rocksdb::FSWritableFile> >*, rocksdb::IODebugContext*) internal_repo_rocksdb/repo/env/fs_posix.cc:334 (external_sst_file_test+0xb61ac4) #1 rocksdb::(anonymous namespace)::PosixFileSystem::ReopenWritableFile(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*) internal_repo_rocksdb/repo/env/fs_posix.cc:382 (external_sst_file_test+0xb5ba96) #2 rocksdb::CompositeEnv::ReopenWritableFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::unique_ptr<rocksdb::WritableFile, std::default_delete<rocksdb::WritableFile> >*, rocksdb::EnvOptions const&) internal_repo_rocksdb/repo/env/composite_env.cc:334 (external_sst_file_test+0xa6ab7f) facebook#3 rocksdb::EnvWrapper::ReopenWritableFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::unique_ptr<rocksdb::WritableFile, std::default_delete<rocksdb::WritableFile> >*, rocksdb::EnvOptions const&) internal_repo_rocksdb/repo/include/rocksdb/env.h:1428 (external_sst_file_test+0x561f3e) Previous read of size 1 at 0x556fede63340 by thread T4: #0 rocksdb::(anonymous namespace)::PosixFileSystem::OpenWritableFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, rocksdb::FileOptions const&, bool, std::unique_ptr<rocksdb::FSWritableFile, std::default_delete<rocksdb::FSWritableFile> >*, rocksdb::IODebugContext*) internal_repo_rocksdb/repo/env/fs_posix.cc:328 (external_sst_file_test+0xb61a70) #1 rocksdb::(anonymous namespace)::PosixFileSystem::ReopenWritableFile(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator ... ``` Fix by making sure the following block gets executed only once: ``` if (!checkedDiskForMmap_) { // this will be executed once in the program's lifetime. // do not use mmapWrite on non ext-3/xfs/tmpfs systems. if (!SupportsFastAllocate(fname)) { forceMmapOff_ = true; } checkedDiskForMmap_ = true; } ``` Reviewed By: pdillinger Differential Revision: D34780308 fbshipit-source-id: b761f66b24c8b5b8389d86ea371c8542b8d869d5
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Is it possible to merge this PR which is the latest commits form master repo and publish an nuget (without the snappy dependency)