Update HISTORY for SeekForPrev bug fix #5925
Conversation
HISTORY.md
Outdated
| @@ -8,6 +8,7 @@ | |||
| * Fix a bug when format_version=3, partitioned fitlers, and prefix search are used in conjunction. The bug could result into Seek::(prefix) returning NotFound for an existing prefix. | |||
| * Revert the feature "Merging iterator to avoid child iterator reseek for some cases (#5286)" since it might cause strong results when reseek happens with a different iterator upper bound. | |||
| * Fix a bug causing a crash during ingest external file when background compaction cause severe error (file not found). | |||
| * Fix a bug when partitioned fitlers and prefix search are used in conjunction, ::SeekForPrev could return invalid for an existing prefix. | |||
There was a problem hiding this comment.
This description is misleading. Users don't need to explicitly call SeekForPrev() to see the bug. It can be called inside RocksDB for other operations. For example, the failure we saw was a Seek() followed by Prev().
There was a problem hiding this comment.
I updated that to include Seek+Prev, and Seek that involves Delete or Merge operands.
HISTORY.md
Outdated
| @@ -8,6 +8,7 @@ | |||
| * Fix a bug when format_version=3, partitioned fitlers, and prefix search are used in conjunction. The bug could result into Seek::(prefix) returning NotFound for an existing prefix. | |||
| * Revert the feature "Merging iterator to avoid child iterator reseek for some cases (#5286)" since it might cause strong results when reseek happens with a different iterator upper bound. | |||
| * Fix a bug causing a crash during ingest external file when background compaction cause severe error (file not found). | |||
| * Fix a bug when partitioned fitlers and prefix search are used in conjunction, ::SeekForPrev could return invalid for an existing prefix. ::SeekForPrev might be called by the user, or internally on ::Seek followed by ::Prev, or within Seek if the return value involves Delete or a Merge operand. | |||
There was a problem hiding this comment.
I don't know whether users got the feeling that Seek + Next + Prev won't have the bug. It will be good to clarify.
There was a problem hiding this comment.
I extended it to all ::Prev calls since Seek does not have be called to be a forward direction iterator.
HISTORY.md
Outdated
| @@ -8,6 +8,7 @@ | |||
| * Fix a bug when format_version=3, partitioned fitlers, and prefix search are used in conjunction. The bug could result into Seek::(prefix) returning NotFound for an existing prefix. | |||
| * Revert the feature "Merging iterator to avoid child iterator reseek for some cases (#5286)" since it might cause strong results when reseek happens with a different iterator upper bound. | |||
| * Fix a bug causing a crash during ingest external file when background compaction cause severe error (file not found). | |||
| * Fix a bug when partitioned fitlers and prefix search are used in conjunction, ::SeekForPrev could return invalid for an existing prefix. ::SeekForPrev might be called by the user, or internally on ::Seek followed by ::Prev, or within Seek if the return value involves Delete or a Merge operand. | |||
There was a problem hiding this comment.
Also it's not within Seek(), Next() can also call it if there is a merge operand.
We may consider to soften the tone so that it may not be a full list.
There was a problem hiding this comment.
By code inspection I could not see how ::Next would end up calling that.
There was a problem hiding this comment.
@maysamyabandeh is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@maysamyabandeh merged this pull request in 2c9e9f2. |
Summary: Update history for the bug fix in facebook#5907 Pull Request resolved: facebook#5925 Differential Revision: D17952605 Pulled By: maysamyabandeh fbshipit-source-id: 609afcbb2e4087f9153822c4d11193a75a7b0e7a
Summary: Update history for the bug fix in facebook#5907 Pull Request resolved: facebook#5925 Differential Revision: D17952605 Pulled By: maysamyabandeh fbshipit-source-id: 609afcbb2e4087f9153822c4d11193a75a7b0e7a
Update history for the bug fix in #5907