HBASE-26762: Un-Deprecate and improve documentation for Scan#setRowPrefixFilter #4119
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
|
I'm in doubt about the test that was added testRowPrefixFilterAndStartRow : This is effectively showing that if you use these functions incorrectly you get unexpected results. |
nielsbasjes
force-pushed
the
HBASE-26762-setRowPrefixFilter
branch
from
fe6c4b9 to
d35632f
Compare
|
As discussed with Duo Zhang I'm re-deprecating the old name and providing a replacement with a better (non-confusing) name: setStartStopRowForPrefixScan In addition to the Scan related method I found that both of these classes also have a method with the same name "setRowPrefixFilter" ./hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALPrettyPrinter.java ./hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/replication/VerifyReplication.java |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
…efixFilter Signed-off-by: Niels Basjes <nielsbasjes@apache.org>
…d results Signed-off-by: Niels Basjes <nielsbasjes@apache.org>
…owForPrefixScan Signed-off-by: Niels Basjes <nielsbasjes@apache.org>
nielsbasjes
force-pushed
the
HBASE-26762-setRowPrefixFilter
branch
from
20500dd to
3129b5f
Compare
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Apache9
approved these changes
Feb 26, 2022
|
Oh, one minor thing, I think we could also cherry-pick this change to branch-2 and branch-2.5, so let's change the javadoc from 'since 3.0.0' to 'since 2.5.0'? Thanks. |
Signed-off-by: Niels Basjes <nielsbasjes@apache.org>
|
I've pushed it with since 2.5.0 now.
Just let me know if I have to do something from here. |
|
Our guideline is to remove a method which has been marked as deprecated for a whole major release, so usually, if a method is deprecated in 2.5.0, we will remove it in 4.0.0. Unless we found that this is a mistake, or the behavior is really confusing, we will remove it more aggressively. For this method, since it has been there for a long time, I tend to keep it there and only remove it in 4.0.0, so you could modify the javadoc to 'since 3.0.0, will be removed in 4.0.0'. And please fix the checkstyle issues? They are both line length issue, a simple format is enough to fix them. Thanks. |
|
🎊 +1 overall
This message was automatically generated. |
Signed-off-by: Niels Basjes <nielsbasjes@apache.org>
I think this should be 'since 2.5.0, will be removed in 4.0.0' ?
Done. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
|
🎊 +1 overall
This message was automatically generated. |
|
Is there anything I can do to fix this last error? |
apurtell
approved these changes
Feb 28, 2022
Apache9
pushed a commit
that referenced
this pull request
Mar 2, 2022
…fixFilter (#4119) Signed-off-by: Andrew Purtell <apurtell@apache.org> Signed-off-by: Duo Zhang <zhangduo@apache.org>
Apache9
pushed a commit
that referenced
this pull request
Mar 2, 2022
…fixFilter (#4119) Signed-off-by: Andrew Purtell <apurtell@apache.org> Signed-off-by: Duo Zhang <zhangduo@apache.org>
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.
Issue https://issues.apache.org/jira/browse/HBASE-26762
I found today that with ticket https://issues.apache.org/jira/browse/HBASE-25299 the intended effects of Scan#setRowPrefixFilter were unclear (hence the report of unexpected effects) and instead of just improving the documentation the method was marked as deprecated.
Quote:
Yes, if you use this very valuable method incorrectly it will yield incorrect results.
I strongly disagree with this deprecation because it is one of the most used methods of Scan in many of my applications.
I do agree the documentation should more clearly indicate these combinations do not work.
This pull request does only 2 things: