Fix bug in MultiRead() coalescing introduced in 4fc216649d (#6446). #6979
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: TryMerge() overzealously created one huge file read request in an attempt to merge smaller disjoint requests. For example, ~30 input requests of ~100 bytes output as 1 request of 100 MiB causing alarmingly large read throughputs to be repeatedly observed by the environment. Signed-off-by: Jason Volk <jason@zemos.net>
mrambacher
approved these changes
Jul 22, 2020
There was a problem hiding this comment.
This change seems valid/logical to me as the original code obviously has a typo.
My only question is if there is a simple test case you can provide that shows a problem before this fix and not after it? A test case would show the bug and probably be a help in getting this merged.
There was a problem hiding this comment.
@Cheng-Chang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@Cheng-Chang merged this pull request in 4a60cb2. |
Myasuka
pushed a commit
to Myasuka/frocksdb
that referenced
this pull request
Oct 22, 2020
Summary: TryMerge() overzealously creates one huge file read request in an attempt to merge smaller disjoint requests. For example, ~30 input requests of ~100 bytes output as 1 request of 100 MiB causing alarmingly large read throughputs to be repeatedly observed by the environment. Signed-off-by: Jason Volk <jason@zemos.net> Pull Request resolved: facebook/rocksdb#6979 Reviewed By: siying Differential Revision: D22668892 Pulled By: cheng-chang fbshipit-source-id: 7506fe9621b7f1a747dadf6b8ddb1b1a141c1937
Myasuka
pushed a commit
to Myasuka/frocksdb
that referenced
this pull request
Oct 23, 2020
Summary: TryMerge() overzealously creates one huge file read request in an attempt to merge smaller disjoint requests. For example, ~30 input requests of ~100 bytes output as 1 request of 100 MiB causing alarmingly large read throughputs to be repeatedly observed by the environment. Signed-off-by: Jason Volk <jason@zemos.net> Pull Request resolved: facebook/rocksdb#6979 Reviewed By: siying Differential Revision: D22668892 Pulled By: cheng-chang fbshipit-source-id: 7506fe9621b7f1a747dadf6b8ddb1b1a141c1937
Myasuka
added a commit
to Myasuka/frocksdb
that referenced
this pull request
Oct 26, 2020
* Fix bug in MultiRead() coalescing introduced in 4fc2166 (#6446). (#6979) Summary: TryMerge() overzealously creates one huge file read request in an attempt to merge smaller disjoint requests. For example, ~30 input requests of ~100 bytes output as 1 request of 100 MiB causing alarmingly large read throughputs to be repeatedly observed by the environment. Signed-off-by: Jason Volk <jason@zemos.net> Pull Request resolved: facebook/rocksdb#6979 Reviewed By: siying Differential Revision: D22668892 Pulled By: cheng-chang fbshipit-source-id: 7506fe9621b7f1a747dadf6b8ddb1b1a141c1937 * [FLINK-10471] Add Apache Flink specific compaction filter to evict expired state which has time-to-live * Extend StringAppendTESTOperator to use string delimiter of variable length (#2) * Frocksdb release guide and helping scripts This patch combines dataArtisans@92a7360 and dataArtisans@810b42a * fixup! [FLINK-10471] Add Apache Flink specific compaction filter to evict expired state which has time-to-live * fixup! Extend StringAppendTESTOperator to use string delimiter of variable length (#2) * fixup! [FLINK-10471] Add Apache Flink specific compaction filter to evict expired state which has time-to-live Co-authored-by: Jason Volk <jason@zemos.net> Co-authored-by: azagrebin <azagrebin@users.noreply.github.com>
Myasuka
pushed a commit
to ververica/frocksdb
that referenced
this pull request
Oct 29, 2020
Summary: TryMerge() overzealously creates one huge file read request in an attempt to merge smaller disjoint requests. For example, ~30 input requests of ~100 bytes output as 1 request of 100 MiB causing alarmingly large read throughputs to be repeatedly observed by the environment. Signed-off-by: Jason Volk <jason@zemos.net> Pull Request resolved: facebook/rocksdb#6979 Reviewed By: siying Differential Revision: D22668892 Pulled By: cheng-chang fbshipit-source-id: 7506fe9621b7f1a747dadf6b8ddb1b1a141c1937
Myasuka
pushed a commit
to Myasuka/frocksdb
that referenced
this pull request
Nov 3, 2020
Summary: TryMerge() overzealously creates one huge file read request in an attempt to merge smaller disjoint requests. For example, ~30 input requests of ~100 bytes output as 1 request of 100 MiB causing alarmingly large read throughputs to be repeatedly observed by the environment. Signed-off-by: Jason Volk <jason@zemos.net> Pull Request resolved: facebook/rocksdb#6979 Reviewed By: siying Differential Revision: D22668892 Pulled By: cheng-chang fbshipit-source-id: 7506fe9621b7f1a747dadf6b8ddb1b1a141c1937
Myasuka
pushed a commit
to Myasuka/frocksdb
that referenced
this pull request
Nov 6, 2020
Summary: TryMerge() overzealously creates one huge file read request in an attempt to merge smaller disjoint requests. For example, ~30 input requests of ~100 bytes output as 1 request of 100 MiB causing alarmingly large read throughputs to be repeatedly observed by the environment. Signed-off-by: Jason Volk <jason@zemos.net> Pull Request resolved: facebook/rocksdb#6979 Reviewed By: siying Differential Revision: D22668892 Pulled By: cheng-chang fbshipit-source-id: 7506fe9621b7f1a747dadf6b8ddb1b1a141c1937
This was referenced Nov 6, 2020
codingrhythm
pushed a commit
to SafetyCulture/rocksdb
that referenced
this pull request
Mar 5, 2021
…. (facebook#6979) Summary: TryMerge() overzealously creates one huge file read request in an attempt to merge smaller disjoint requests. For example, ~30 input requests of ~100 bytes output as 1 request of 100 MiB causing alarmingly large read throughputs to be repeatedly observed by the environment. Signed-off-by: Jason Volk <jason@zemos.net> Pull Request resolved: facebook#6979 Reviewed By: siying Differential Revision: D22668892 Pulled By: cheng-chang fbshipit-source-id: 7506fe9621b7f1a747dadf6b8ddb1b1a141c1937
levichen94
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 18, 2021
Summary: TryMerge() overzealously creates one huge file read request in an attempt to merge smaller disjoint requests. For example, ~30 input requests of ~100 bytes output as 1 request of 100 MiB causing alarmingly large read throughputs to be repeatedly observed by the environment. Signed-off-by: Jason Volk <jason@zemos.net> Pull Request resolved: facebook/rocksdb#6979 Reviewed By: siying Differential Revision: D22668892 Pulled By: cheng-chang fbshipit-source-id: 7506fe9621b7f1a747dadf6b8ddb1b1a141c1937 Signed-off-by: Changlong Chen <levisonchen@live.cn>
mm304321141
pushed a commit
to bytedance/terarkdb
that referenced
this pull request
Jun 23, 2021
Summary: TryMerge() overzealously creates one huge file read request in an attempt to merge smaller disjoint requests. For example, ~30 input requests of ~100 bytes output as 1 request of 100 MiB causing alarmingly large read throughputs to be repeatedly observed by the environment. Signed-off-by: Jason Volk <jason@zemos.net> Pull Request resolved: facebook/rocksdb#6979 Reviewed By: siying Differential Revision: D22668892 Pulled By: cheng-chang fbshipit-source-id: 7506fe9621b7f1a747dadf6b8ddb1b1a141c1937 Signed-off-by: Changlong Chen <levisonchen@live.cn>
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.
TryMerge() overzealously creates one huge file read request in an attempt to merge smaller disjoint requests. For example, ~30 input requests of ~100 bytes output as 1 request of 100 MiB causing alarmingly large read throughputs to be repeatedly observed by the environment.
Signed-off-by: Jason Volk jason@zemos.net