facebook-github-bot Github contribution chart
facebook-github-bot Github Stats
facebook-github-bot Most Used Languages

Activity

06 Dec 2022

Issue Comment

Facebook-github-bot

[Delta] aspeed: gpio: Add ast2600 gpio table.

Hi all,

I noticed that you don't have an ast2600 GPIO table, so I added ast2600-gpio.csv according to ASPEED ast2620 datasheet and converted it to ast2600_gpio_table.py by the parser you provided. Also, I changed the value of SCU_REG_MAX from 0x1A4 to 0x6E8 to fix the issue of parsing error while booting. Last, I added ast2600_gpio_table.py in openbmc-gpio_0.2.bbappend and built it successfully. This commit can be built and booted successfully. All changes are under the meta-aspeed directory.

Thanks, Delta

Forked On 06 Dec 2022 at 03:26:26

Facebook-github-bot

Hi @ChuTingI!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

Commented On 06 Dec 2022 at 03:26:26

Facebook-github-bot

Centrally handle Rust toolchain attr defaults

Summary: This reverts the Rust parts from D41714656 (https://github.com/facebookincubator/buck2/commit/f171bf114737888cc10471f9681bc167096ae305) and implements it better a different way.

Advantages of the new approach:

  1. No more "falsiness". The default value kicks in only if a value was not passed by the code that instantiated the toolchain, or if None was passed. (As far as I can tell there is no way to tell these cases apart, but that seems fine.) Previously we had been doing e.g. toolchain_info.rustc_flags or [], which will silently accept a toolchain constructed with inappropriate falsey values like rustc_flags = False or rustc_flags = "".

  2. A consistent central place for the default values. No more needing to scatter or [] to every location that a particular attribute is referenced.

  3. A central place to observe which attributes have not yet been given a default value, and discuss what those default values should be.

Other than #1 above, this diff does not intentionally change any behavior.

Reviewed By: zertosh

Differential Revision: D41752388

fbshipit-source-id: 47e8b290cc596528a7a3c5b3a68195083725f8bd

Pushed On 06 Dec 2022 at 03:15:20

Facebook-github-bot

Introduce a new constraint fully_qualified_name

Summary: For performance optimizations, we want to introduce different constraints on the name and the fully qualified name (i.e, including modules) of a function, method or class.

This diff adds the constraints fully_qualified_name.equals, fully_qualified_name.matches, cls.fully_qualified_name.equals, cls.fully_qualified_name.matches. For now, these have the same behavior as their counterpart name.equals, etc. In the future, name.equals will only match on the function name or method name, and cls.name will only match on the class name, excluding module names.

Reviewed By: alexkassil

Differential Revision: D41708125

fbshipit-source-id: f211e72109d216d59227d1edac2249ce334dfadf

Pushed On 06 Dec 2022 at 03:15:04

Facebook-github-bot

Centrally handle Rust toolchain attr defaults

Summary: This reverts the Rust parts from D41714656 (https://github.com/facebookincubator/buck2-prelude/commit/fad02442f4c9cbcaabb32d032daac7b759602faf) and implements it better a different way.

Advantages of the new approach:

  1. No more "falsiness". The default value kicks in only if a value was not passed by the code that instantiated the toolchain, or if None was passed. (As far as I can tell there is no way to tell these cases apart, but that seems fine.) Previously we had been doing e.g. toolchain_info.rustc_flags or [], which will silently accept a toolchain constructed with inappropriate falsey values like rustc_flags = False or rustc_flags = "".

  2. A consistent central place for the default values. No more needing to scatter or [] to every location that a particular attribute is referenced.

  3. A central place to observe which attributes have not yet been given a default value, and discuss what those default values should be.

Other than #1 above, this diff does not intentionally change any behavior.

Reviewed By: zertosh

Differential Revision: D41752388

fbshipit-source-id: 47e8b290cc596528a7a3c5b3a68195083725f8bd

Pushed On 06 Dec 2022 at 03:14:26
Issue Comment

Facebook-github-bot

Update flake lock file

Automated changes by create-pull-request GitHub action

Forked On 06 Dec 2022 at 03:06:13

Facebook-github-bot

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Commented On 06 Dec 2022 at 03:06:13
Issue Comment

Facebook-github-bot

Update flake lock file

Automated changes by create-pull-request GitHub action

Forked On 06 Dec 2022 at 03:05:59

Facebook-github-bot

@github-actions[bot] has updated the pull request. You must reimport the pull request before landing.

Commented On 06 Dec 2022 at 03:05:59
Issue Comment

Facebook-github-bot

Add simulate_rir_ism method for room impulse response simulation

replicate of #2644

Forked On 06 Dec 2022 at 03:05:00

Facebook-github-bot

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

Commented On 06 Dec 2022 at 03:05:00

Facebook-github-bot

suppress errors in buck2/prelude/python/tools

Differential Revision: D41755597

fbshipit-source-id: b6f0f1bf233ae78967b24c6785e53a6fd22d005c

Pushed On 06 Dec 2022 at 02:56:44

Facebook-github-bot

suppress errors in buck2/prelude/python/tools

Differential Revision: D41755597

fbshipit-source-id: b6f0f1bf233ae78967b24c6785e53a6fd22d005c

Pushed On 06 Dec 2022 at 02:56:24

Facebook-github-bot

Updating submodules

Summary: GitHub commits:

https://github.com/facebook/fbthrift/commit/8896067864408a05907ba9676136822ef3a38af3 https://github.com/facebook/proxygen/commit/a520aa24a0ed1d1229cc18ec1517f68f2955e0b2 https://github.com/facebook/wangle/commit/916c59490ac796e1c41e20c9bc18b1acbe0dd66c https://github.com/facebook/watchman/commit/7df58ddce35ad801398113da8f027d8c1fd5d628 https://github.com/facebookexperimental/edencommon/commit/7dd4fed459f86e373c203973c786b45554f850c6 https://github.com/facebookexperimental/rust-shed/commit/02119f6f5f5e67a90baaa247748fe2f9f93896d9 https://github.com/facebookincubator/fizz/commit/e62abe08ce64d2dec58194b30e2daf56294cc6b9 https://github.com/facebookincubator/katran/commit/0b5f8e1f87e48de03fa046ab7276d0c097a39c80 https://github.com/facebookincubator/mvfst/commit/aecff0f68bb288ad331269bb164ab5d8917d985c https://github.com/facebookincubator/velox/commit/0eebc81042894233416bb53f93b6a0a20faa9aaf https://github.com/pytorch/fbgemm/commit/bdccbcfc3038d2a30008125432b4f72102c00c69

Reviewed By: jailby

fbshipit-source-id: 508a1fb4f5b40ca5aac15e66e0acb5e6ab3053bc

Pushed On 06 Dec 2022 at 02:50:56

Facebook-github-bot

acl entry as thrift cow struct node

Summary: as titled

Reviewed By: peygar

Differential Revision: D41591537

LaMa Project: L1125642

fbshipit-source-id: f7dd4ae1f99aa56777ffc52c6810528ea1c7c288

Pushed On 06 Dec 2022 at 02:50:56

Facebook-github-bot

migrate acl map to thrift cow

Summary: as titled

Reviewed By: peygar

Differential Revision: D41660861

LaMa Project: L1125642

fbshipit-source-id: 988ba61f6efc1de1630d65576c2b3898e6ba5676

Pushed On 06 Dec 2022 at 02:50:56

Facebook-github-bot

remove unused method from acl table

Summary: as titled

Reviewed By: peygar

Differential Revision: D41662327

LaMa Project: L1125642

fbshipit-source-id: 8bcafa9fef44d62d0672668bc2ff0b1113ee8cde

Pushed On 06 Dec 2022 at 02:50:56

Facebook-github-bot

migrate acl table map to thrift cow

Summary: as titled

Reviewed By: peygar

Differential Revision: D41665107

LaMa Project: L1125642

fbshipit-source-id: dbbabe191c8bbb1ef7cfe9633a0f004b54e20ed5

Pushed On 06 Dec 2022 at 02:50:56
Issue Comment

Facebook-github-bot

Suppress dtype warnings in unit tests

Summary: The dtype warnings were being raised a few too many times when running unit tests. This adds a filter to ignore them.

Differential Revision: D41757594

Forked On 06 Dec 2022 at 02:50:35

Facebook-github-bot

This pull request was exported from Phabricator. Differential Revision: D41757594

Commented On 06 Dec 2022 at 02:50:35

Facebook-github-bot

Fix std::move in PyBuffer.cpp

Summary: std:: is required before move in LLVM-15

Reviewed By: georges-berenger

Differential Revision: D41737725

fbshipit-source-id: 63bdc217e99f5cbcf1f6ac1bed699426e708fb6a

Pushed On 06 Dec 2022 at 02:50:35

Facebook-github-bot

Add large chunk memory allocations support for memory pool

Add large chunk memory allocations for memory pool and deprecate ScopedMemoryAllocator. Correspondingly, removed all the direct memory allocator usages in Velox and all the large chunk memory allocations now go through memory pool interface. This PR also improve the management of Allocation and ContiguousAllocation objects by removing the allocator from the object constructor. The memory allocator doesn't own or are aware of the ownerships of the two allocation object types. It is memory pool object to set the pool or ownership on allocation success. For direct allocation from memory allocator such as AsyncDataCache, then user needs to free the allocations explicitly. Add unit tests for the new memory pool interfaces and thorough testing on the two allocation objects as well.

Forked On 06 Dec 2022 at 02:49:43

Facebook-github-bot

This pull request was exported from Phabricator. Differential Revision: D41725660

Commented On 06 Dec 2022 at 02:49:43

Facebook-github-bot

Faster Expression.name_to_reference implementation

Summary: The current name_to_reference implementation is not tail-recursive and requires an additional List.rev. What's funny is that, just above it, we have a tail-recursive name_to_identifiers implementation. We can just re-use that.

Reviewed By: jasontatton-meta

Differential Revision: D41694400

fbshipit-source-id: 9a7d19ae1cfc13c56bce72244841463caa2ac836

Pushed On 06 Dec 2022 at 02:41:43

Facebook-github-bot

Turn the callee into a reference when parsing constraint

Summary: All valid constraints have the form reference(arguments). Because of this, we can pattern match on a Call { callee; arguments } and convert callee into a reference, before pattern matching. This simplifies a lot the parsing of constraints.

Reviewed By: litho17

Differential Revision: D41700694

fbshipit-source-id: 24de2c2d8babb7e36b211758711abba085eec04c

Pushed On 06 Dec 2022 at 02:41:43

Facebook-github-bot

Factor out class constraint parsing

Summary: The logic to parse class constraint was duplicated in two places. This diff fixes it.

Reviewed By: litho17

Differential Revision: D41704008

fbshipit-source-id: 31beea58bafb0b2e1d8428605078843bceabbb59

Pushed On 06 Dec 2022 at 02:41:43

Facebook-github-bot

Rename the constraint cls.equals into cls.name.equals

Summary: For performance optimizations, we want to introduce different constraints on the name and the fully qualified name (i.e, including modules) of a function, method or class.

This is not currently possible for class constraints since the constraint is called cls.equals/cls.matches. To fix this problem, we introduce in this diff cls.name.equals and cls.name.matches with the same semantic. We will then deprecate cls.equals in the future.

Reviewed By: litho17

Differential Revision: D41704438

fbshipit-source-id: 64ae4343ec6036f9e5fae664a44ee683df921eb0

Pushed On 06 Dec 2022 at 02:41:43

Facebook-github-bot

Add repeat Presto function

Adding the repeat Presto function to Velox.

repeat(element, count) → array:      - Repeat element for count times.

Examples:

SELECT repeat(1, 3); -- [1, 1, 1]
SELECT repeat(1, 0); -- []
SELECT repeat("foo", 2); -- ["foo", "foo"]
SELECT repeat(true, 3); -- [true, true, true]
SELECT repeat(null, 2); -- [null, null]
SELECT repeat(3, null); -- null

# illegal count argument
SELECT repeat(10, -2);
SELECT repeat(10, 1.23);
SELECT repeat(10, "foo");
SELECT repeat(10, true); 

Forked On 06 Dec 2022 at 02:40:42

Facebook-github-bot

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

Commented On 06 Dec 2022 at 02:40:42

Facebook-github-bot

refactor(react-native/polyfills): rename package to @react-native/js-polyfills and align version

Summary: Changelog: [General][Changed] - renamed react-native/polyfills -> react-native/js-polyfills and align with other packages versions (0.72.0) as a part of migration to monorepo

Reviewed By: motiz88

Differential Revision: D41553157

fbshipit-source-id: eef70c8e7639080acdeb6716d1a915760a85458a

Pushed On 06 Dec 2022 at 02:38:54

Facebook-github-bot

npm security updates as recommended by GitHub dependabot

Summary: This rolls the following PRs generated by dependabot into one:

https://github.com/facebook/sapling/pull/136 https://github.com/facebook/sapling/pull/137 https://github.com/facebook/sapling/pull/301

Reviewed By: sggutier

Differential Revision: D41752254

fbshipit-source-id: 3f21cc0cf57d29487d8bfe5222d11cd9e77c8db7

Pushed On 06 Dec 2022 at 02:37:25
Issue Comment

Facebook-github-bot

Potentially deflake DBWALTest.FixSyncWalOnObseletedWalWithNewManifestCausingMissingWAL

Context/Summary: Credit to @ajkr's https://github.com/facebook/rocksdb/pull/11016#pullrequestreview-1205020134, flaky test https://app.circleci.com/pipelines/github/facebook/rocksdb/21985/workflows/5f6cc355-78c1-46d8-89ee-0fd679725a8a/jobs/540878 is due to Flush() called in the test returned earlier than obsoleted WAL being found in background flush and SyncWAL() was called (i.e, "sync_point_called" sets to true). Fix this by making checking sync_point_called == true after obsoleted WAL is found and SyncWAL() is called. Also rename the "sync_point_called" to be something more specific.

Also, fix a potential flakiness due to manually setting a log threshold to force new manifest creation. This is unreliable so I decided to use sync point to force new manifest creation.

Test plan: make check

Forked On 06 Dec 2022 at 02:36:15

Facebook-github-bot

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

Commented On 06 Dec 2022 at 02:36:15
Issue Comment

Facebook-github-bot

Potentially deflake DBWALTest.FixSyncWalOnObseletedWalWithNewManifestCausingMissingWAL

Context/Summary: I accidentally observed this test failure https://app.circleci.com/pipelines/github/facebook/rocksdb/21985/workflows/5f6cc355-78c1-46d8-89ee-0fd679725a8a/jobs/540878 that can't be repro-ed. It belongs to a recent PR of mine https://github.com/facebook/rocksdb/pull/10892/files, which manually sets a log threshold to force new manifest creation. This is unreliable so I decided to use sync point to force new manifest creation.

Test plan: make check

Forked On 06 Dec 2022 at 02:32:48

Facebook-github-bot

@hx235 has updated the pull request. You must reimport the pull request before landing.

Commented On 06 Dec 2022 at 02:32:48
Issue Comment

Facebook-github-bot

Potentially deflake DBWALTest.FixSyncWalOnObseletedWalWithNewManifestCausingMissingWAL

Context/Summary: I accidentally observed this test failure https://app.circleci.com/pipelines/github/facebook/rocksdb/21985/workflows/5f6cc355-78c1-46d8-89ee-0fd679725a8a/jobs/540878 that can't be repro-ed. It belongs to a recent PR of mine https://github.com/facebook/rocksdb/pull/10892/files, which manually sets a log threshold to force new manifest creation. This is unreliable so I decided to use sync point to force new manifest creation.

Test plan: make check

Forked On 06 Dec 2022 at 02:32:19

Facebook-github-bot

@hx235 has updated the pull request. You must reimport the pull request before landing.

Commented On 06 Dec 2022 at 02:32:19
Issue Comment

Facebook-github-bot

Potentially deflake DBWALTest.FixSyncWalOnObseletedWalWithNewManifestCausingMissingWAL

Context/Summary: I accidentally observed this test failure https://app.circleci.com/pipelines/github/facebook/rocksdb/21985/workflows/5f6cc355-78c1-46d8-89ee-0fd679725a8a/jobs/540878 that can't be repro-ed. It belongs to a recent PR of mine https://github.com/facebook/rocksdb/pull/10892/files, which manually sets a log threshold to force new manifest creation. This is unreliable so I decided to use sync point to force new manifest creation.

Test plan: make check

Forked On 06 Dec 2022 at 02:25:13

Facebook-github-bot

@hx235 has updated the pull request. You must reimport the pull request before landing.

Commented On 06 Dec 2022 at 02:25:13

Facebook-github-bot

fix mmdet detector wrapper init

Created On 06 Dec 2022 at 02:12:30

Facebook-github-bot

fix mmdet detector wrapper init

Summary: MMDet Detector wrapper currently doesn't call init

Pull Request resolved: https://github.com/facebookresearch/detectron2/pull/4684

Reviewed By: tglik

Differential Revision: D41710622

fbshipit-source-id: 758834484a8e4d188e1b0baba72a9574416cd5d9

Pushed On 06 Dec 2022 at 02:12:25

Facebook-github-bot

Create OrderedSet with known capacity in get_configured_target_node_no_transition

Summary: When creating the OrderedSets for deps and exec_deps, we already know how large they are going to be, so allocate them with capacity set.

Also, initialize these OrderedSets earlier so that we can drop previously allocated deps and exec_deps early.

Reviewed By: bobyangyf

Differential Revision: D41744062

fbshipit-source-id: 64f49efbb5a1f056478bc4284366e14500fd6056

Pushed On 06 Dec 2022 at 02:09:47
Issue Comment

Facebook-github-bot

use apply_optimizer_in_backward in two tower retrieval example

Summary: as title

Differential Revision: D41756811

Forked On 06 Dec 2022 at 02:08:15

Facebook-github-bot

This pull request was exported from Phabricator. Differential Revision: D41756811

Commented On 06 Dec 2022 at 02:08:15
Issue Comment

Facebook-github-bot

Sort L0 files by newly introduced epoch_num

Context: Sorting L0 file by largest_seqno has at least two inconvenience:

  • File ingestion and compaction involving ingested files can create files of overlapping seqno range with the existing files. force_consistency_check=true will catch such overlap seqno range even those harmless overlap.
    • For example, consider the following sequence of events ("key@n" indicates key at seqno "n")
      • insert k1@1 to memtable m1
      • ingest file s1 with k2@2, ingest file s2 with k3@3
      • insert k4@4 to m1
      • compact files s1, s2 and result in new file s3 of seqno range [2, 3]
      • flush m1 and result in new file s4 of seqno range [1, 4]. And force_consistency_check=true will think s4 and s3 has file reordering corruption that might cause retuning an old value of k1
    • However such caught corruption is a false positive since s1, s2 will not have overlapped keys with k1 or whatever inserted into m1 before ingest file s1 by the requirement of file ingestion (otherwise the m1 will be flushed first before any of the file ingestion completes). Therefore there in fact isn't any file reordering corruption.
  • Single delete can decrease a file's largest seqno and ordering by largest_seqno can introduce a wrong ordering hence file reordering corruption
    • For example, consider the following sequence of events ("key@n" indicates key at seqno "n", Credit to @ajkr for this example)
      • an existing SST s1 contains only k1@1
      • insert k1@2 to memtable m1
      • ingest file s2 with k3@3, ingest file s3 with k4@4
      • insert single delete k5@5 in m1
      • flush m1 and result in new file s4 of seqno range [2, 5]
      • compact s1, s2, s3 and result in new file s5 of seqno range [1, 4]
      • compact s4 and result in new file s6 of seqno range [2] due to single delete
    • By the last step, we have file ordering by largest seqno (">" means "newer") : s5 > s6 while s6 contains a newer version of the k1's value (i.e, k1@2) than s5, which is a real reordering corruption. While this can be caught by force_consistency_check=true, there isn't a good way to prevent this from happening if ordering by largest_seqno

Therefore, we are redesigning the sorting criteria of L0 files and avoid above inconvenience. Credit to @ajkr , we now introduce epoch_num which describes the order of a file being flushed or ingested/imported (compaction output file will has the minimum epoch_num among input files'). This will avoid the above inconvenience in the following ways:

  • In the first case above, there will no longer be overlap seqno range check in force_consistency_check=true but epoch_number ordering check. This will result in file ordering s1 < s2 < s4 (pre-compaction) and s3 < s4 (post-compaction) which won't trigger false positive corruption. See test class DBCompactionTestL0FilesMisorderCorruption* for more.
  • In the second case above, this will result in file ordering s1 < s2 < s3 < s4 (pre-compacting s1, s2, s3), s5 < s4 (post-compacting s1, s2, s3), s5 < s6 (post-compacting s4), which are correct file ordering without causing any corruption.

Summary:

  • Introduce epoch_number stored per ColumnFamilyData and sort CF's L0 files by their assigned epoch_number instead of largest_seqno.
    • epoch_number is increased and assigned upon VersionEdit::AddFile() for flush (or similarly for WriteLevel0TableForRecovery) and file ingestion.
    • Compaction output file is assigned with the minimum epoch_number among input files'
      • Refit level: reuse refitted file's epoch_number
    • Other paths needing epoch_number treatment:
      • Import column families: reuse file's epoch_number if exists. If not, assign one based on NewestFirstBySeqNo
      • Repair: reuse file's epoch_number if exists. If not, assign one based on NewestFirstBySeqNo.
    • Assigning new epoch_number to a file and adding this file to LSM tree should be atomic. This is guaranteed by us assigning epoch_number right upon VersionEdit::AddFile() where this version edit will be apply to LSM tree shape right after by holding the db mutex (e.g, flush, file ingestion, import column family) or by there is only 1 ongoing edit per CF (e.g, WriteLevel0TableForRecovery, Repair).
    • Assigning the minimum input epoch number to compaction output file won't misorder L0 files (even through later Refit(target_level=0)). It's due to for every key "k" in the input range, a legit compaction will cover a continuous epoch number range of that key. As long as we assign the key "k" the minimum input epoch number, it won't become newer or older than the versions of this key that aren't included in this compaction hence no misorder.
  • Persist epoch_number of each file in manifest and recover epoch_number on db recovery
    • Backward compatibility with old db without epoch_number support is guaranteed by assigning epoch_number to recovered files by NewestFirstBySeqno order. See VersionSet::RecoverEpochNumbers() for more
    • Forward compatibility with manifest is guaranteed by flexibility of NewFileCustomTag
  • Replace force_consistent_check on L0 with epoch_number and remove false positive check like case 1 with largest_seqno above
    • Due to backward compatibility issue, we might encounter files with missing epoch number at the beginning of db recovery. We will still use old L0 sorting mechanism (NewestFirstBySeqno) to check/sort them till we infer their epoch number. See EpochNumberRequirement.
  • Remove fix https://github.com/facebook/rocksdb/pull/5958#issue-511150930 and their outdated tests to file reordering corruption because such fix can be replaced by this PR.
  • Misc:
    • update existing tests with epoch_number so make check will pass
    • update https://github.com/facebook/rocksdb/pull/5958#issue-511150930 tests to verify corruption is fixed using epoch_number and cover universal/fifo compaction/CompactRange/CompactFile cases
    • assert db_mutex is held for a few places before calling VersionSet::NewEpochNumber()

Test:

  • make check
  • New unit tests under db/db_compaction_test.cc, db/db_test2.cc, db/version_builder_test.cc, db/repair_test.cc
  • Updated tests (i.e, DBCompactionTestL0FilesMisorderCorruption*) under https://github.com/facebook/rocksdb/pull/5958#issue-511150930
  • [Ongoing] Compatibility test: manually run https://github.com/ajkr/rocksdb/commit/36a5686ec012f35a4371e409aa85c404ca1c210d (with file ingestion off for running the .orig binary to prevent this bug affecting upgrade/downgrade formality checking) for 1 hour on simple black/white box, cf_consistency/txn/enable_ts with whitebox + test_best_efforts_recovery with blackbox
  • [Ongoing] normal db stress test
  • [Ongoing] db stress test with aggressive value https://github.com/facebook/rocksdb/pull/10761

Forked On 06 Dec 2022 at 02:07:27

Facebook-github-bot

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

Commented On 06 Dec 2022 at 02:07:27
Issue Comment

Facebook-github-bot

Sort L0 files by newly introduced epoch_num

Context: Sorting L0 file by largest_seqno has at least two inconvenience:

  • File ingestion and compaction involving ingested files can create files of overlapping seqno range with the existing files. force_consistency_check=true will catch such overlap seqno range even those harmless overlap.
    • For example, consider the following sequence of events ("key@n" indicates key at seqno "n")
      • insert k1@1 to memtable m1
      • ingest file s1 with k2@2, ingest file s2 with k3@3
      • insert k4@4 to m1
      • compact files s1, s2 and result in new file s3 of seqno range [2, 3]
      • flush m1 and result in new file s4 of seqno range [1, 4]. And force_consistency_check=true will think s4 and s3 has file reordering corruption that might cause retuning an old value of k1
    • However such caught corruption is a false positive since s1, s2 will not have overlapped keys with k1 or whatever inserted into m1 before ingest file s1 by the requirement of file ingestion (otherwise the m1 will be flushed first before any of the file ingestion completes). Therefore there in fact isn't any file reordering corruption.
  • Single delete can decrease a file's largest seqno and ordering by largest_seqno can introduce a wrong ordering hence file reordering corruption
    • For example, consider the following sequence of events ("key@n" indicates key at seqno "n", Credit to @ajkr for this example)
      • an existing SST s1 contains only k1@1
      • insert k1@2 to memtable m1
      • ingest file s2 with k3@3, ingest file s3 with k4@4
      • insert single delete k5@5 in m1
      • flush m1 and result in new file s4 of seqno range [2, 5]
      • compact s1, s2, s3 and result in new file s5 of seqno range [1, 4]
      • compact s4 and result in new file s6 of seqno range [2] due to single delete
    • By the last step, we have file ordering by largest seqno (">" means "newer") : s5 > s6 while s6 contains a newer version of the k1's value (i.e, k1@2) than s5, which is a real reordering corruption. While this can be caught by force_consistency_check=true, there isn't a good way to prevent this from happening if ordering by largest_seqno

Therefore, we are redesigning the sorting criteria of L0 files and avoid above inconvenience. Credit to @ajkr , we now introduce epoch_num which describes the order of a file being flushed or ingested/imported (compaction output file will has the minimum epoch_num among input files'). This will avoid the above inconvenience in the following ways:

  • In the first case above, there will no longer be overlap seqno range check in force_consistency_check=true but epoch_number ordering check. This will result in file ordering s1 < s2 < s4 (pre-compaction) and s3 < s4 (post-compaction) which won't trigger false positive corruption. See test class DBCompactionTestL0FilesMisorderCorruption* for more.
  • In the second case above, this will result in file ordering s1 < s2 < s3 < s4 (pre-compacting s1, s2, s3), s5 < s4 (post-compacting s1, s2, s3), s5 < s6 (post-compacting s4), which are correct file ordering without causing any corruption.

Summary:

  • Introduce epoch_number stored per ColumnFamilyData and sort CF's L0 files by their assigned epoch_number instead of largest_seqno.
    • epoch_number is increased and assigned upon VersionEdit::AddFile() for flush (or similarly for WriteLevel0TableForRecovery) and file ingestion.
    • Compaction output file is assigned with the minimum epoch_number among input files'
      • Refit level: reuse refitted file's epoch_number
    • Other paths needing epoch_number treatment:
      • Import column families: reuse file's epoch_number if exists. If not, assign one based on NewestFirstBySeqNo
      • Repair: reuse file's epoch_number if exists. If not, assign one based on NewestFirstBySeqNo.
    • Assigning new epoch_number to a file and adding this file to LSM tree should be atomic. This is guaranteed by us assigning epoch_number right upon VersionEdit::AddFile() where this version edit will be apply to LSM tree shape right after by holding the db mutex (e.g, flush, file ingestion, import column family) or by there is only 1 ongoing edit per CF (e.g, WriteLevel0TableForRecovery, Repair).
    • Assigning the minimum input epoch number to compaction output file won't misorder L0 files (even through later Refit(target_level=0)). It's due to for every key "k" in the input range, a legit compaction will cover a continuous epoch number range of that key. As long as we assign the key "k" the minimum input epoch number, it won't become newer or older than the versions of this key that aren't included in this compaction hence no misorder.
  • Persist epoch_number of each file in manifest and recover epoch_number on db recovery
    • Backward compatibility with old db without epoch_number support is guaranteed by assigning epoch_number to recovered files by NewestFirstBySeqno order. See VersionSet::RecoverEpochNumbers() for more
    • Forward compatibility with manifest is guaranteed by flexibility of NewFileCustomTag
  • Replace force_consistent_check on L0 with epoch_number and remove false positive check like case 1 with largest_seqno above
    • Due to backward compatibility issue, we might encounter files with missing epoch number at the beginning of db recovery. We will still use old L0 sorting mechanism (NewestFirstBySeqno) to check/sort them till we infer their epoch number. See EpochNumberRequirement.
  • Remove fix https://github.com/facebook/rocksdb/pull/5958#issue-511150930 and their outdated tests to file reordering corruption because such fix can be replaced by this PR.
  • Misc:
    • update existing tests with epoch_number so make check will pass
    • update https://github.com/facebook/rocksdb/pull/5958#issue-511150930 tests to verify corruption is fixed using epoch_number and cover universal/fifo compaction/CompactRange/CompactFile cases
    • assert db_mutex is held for a few places before calling VersionSet::NewEpochNumber()

Test:

  • make check
  • New unit tests under db/db_compaction_test.cc, db/db_test2.cc, db/version_builder_test.cc, db/repair_test.cc
  • Updated tests (i.e, DBCompactionTestL0FilesMisorderCorruption*) under https://github.com/facebook/rocksdb/pull/5958#issue-511150930
  • [Ongoing] Compatibility test: manually run https://github.com/ajkr/rocksdb/commit/36a5686ec012f35a4371e409aa85c404ca1c210d (with file ingestion off for running the .orig binary to prevent this bug affecting upgrade/downgrade formality checking) for 1 hour on simple black/white box, cf_consistency/txn/enable_ts with whitebox + test_best_efforts_recovery with blackbox
  • [Ongoing] normal db stress test
  • [Ongoing] db stress test with aggressive value https://github.com/facebook/rocksdb/pull/10761

Forked On 06 Dec 2022 at 02:06:53

Facebook-github-bot

@hx235 has updated the pull request. You must reimport the pull request before landing.

Commented On 06 Dec 2022 at 02:06:53

Facebook-github-bot

Updating submodules

Summary: GitHub commits:

https://github.com/facebook/fbthrift/commit/8896067864408a05907ba9676136822ef3a38af3 https://github.com/facebook/proxygen/commit/a520aa24a0ed1d1229cc18ec1517f68f2955e0b2 https://github.com/facebook/wangle/commit/916c59490ac796e1c41e20c9bc18b1acbe0dd66c https://github.com/facebook/watchman/commit/7df58ddce35ad801398113da8f027d8c1fd5d628 https://github.com/facebookexperimental/edencommon/commit/7dd4fed459f86e373c203973c786b45554f850c6 https://github.com/facebookexperimental/rust-shed/commit/02119f6f5f5e67a90baaa247748fe2f9f93896d9 https://github.com/facebookincubator/fizz/commit/e62abe08ce64d2dec58194b30e2daf56294cc6b9 https://github.com/facebookincubator/katran/commit/0b5f8e1f87e48de03fa046ab7276d0c097a39c80 https://github.com/facebookincubator/mvfst/commit/aecff0f68bb288ad331269bb164ab5d8917d985c https://github.com/facebookincubator/velox/commit/0eebc81042894233416bb53f93b6a0a20faa9aaf https://github.com/pytorch/fbgemm/commit/bdccbcfc3038d2a30008125432b4f72102c00c69

Reviewed By: jailby

fbshipit-source-id: 508a1fb4f5b40ca5aac15e66e0acb5e6ab3053bc

Pushed On 06 Dec 2022 at 02:02:44

Facebook-github-bot

Expand and remove the fetch_lm_links function

Summary: In the lm commands, we fetch the link information through the function fetch_lm_links, which does only one thing that is to call client.getInterfaces(). We could avoid the function by directly awaiting that coroutine.

Reviewed By: cenzhao

Differential Revision: D41751585

fbshipit-source-id: 12ad8a8e2d64b8c897c51d5d8d66c05690cc343c

Pushed On 06 Dec 2022 at 02:02:44

Facebook-github-bot

Update Get Started Manually Tutorial

Created On 06 Dec 2022 at 02:01:38

Facebook-github-bot

Update ISSUE_TEMPLATEs

Created On 06 Dec 2022 at 02:01:37