HBASE-25193: Add support for row prefix and type in the WAL Pretty Printer #2555
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
sandeepvinayak
force-pushed
the
HBASE-25193
branch
from
ce592e9
to
26d9b46
Compare
|
@bharathv @virajjasani @wchevreuil Can you please review this? |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
wchevreuil
reviewed
Oct 19, 2020
There was a problem hiding this comment.
This is great, but I wonder if we should have it as an extra option, rather then modifying the current filter for exact equality. That way, we wouldn't break compatibility and we would have extra flexibility for scenarios where it might still be interesting to check for the exact row key.
|
@wchevreuil I believe the nature of prefix doesn't break the backward compatibility. If some one wants exact match just pass the full key in this option as prefix. |
|
I was taking look at #2556 , @sandeepvinayak it looks like you mentioned If we want to go with this current approach (I think it might be enough to have single option), we can document option description to ensure user understands that |
It's not the same, if I have a dataset where rowkeys can be 'a', 'aa', 'aaa' and so on, but I want to find only the edits for rowkey 'a', the |
|
I think we should better provide another option. It's true that we don't want more rows than we are asking for. |
sandeepvinayak
changed the title
|
💔 -1 overall
This message was automatically generated. |
bharathv
approved these changes
Oct 22, 2020
| @@ -101,7 +107,7 @@ | |||
| * Basic constructor that simply initializes values to reasonable defaults. | |||
| */ | |||
| public WALPrettyPrinter() { | |||
| this(false, false, -1, new HashSet<String>(), null, null, false, false, System.out); | |||
| this(false, false, -1, new HashSet<String>(), null, null, null, false, false, System.out); | |||
There was a problem hiding this comment.
I think its time to use builder pattern for this, its getting annoyingly long (perhaps in another patch).
|
🎊 +1 overall
This message was automatically generated. |
virajjasani
pushed a commit
that referenced
this pull request
Oct 23, 2020
…inter Closes #2555 Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Signed-off-by: Bharath Vissapragada <bharathv@apache.org> Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Viraj Jasani <vjasani@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.
Currently, the WAL Pretty Printer has an option to filter the keys with an exact match of row. However, it is super useful sometimes to have a row key prefix instead of an exact match.
The prefix can act as a full match filter as well due to the nature of the prefix.
Additionally, we are not having the cell type in the WAL Pretty Printer in any of the branches.