You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Apache9 (via GitHub)" <gi...@apache.org> on 2023/02/08 10:58:07 UTC
[GitHub] [hbase] Apache9 opened a new pull request, #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Apache9 opened a new pull request, #5016:
URL: https://github.com/apache/hbase/pull/5016
…compressed WAL file
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1425499422
Maybe the problem is that, in replication, we will check whether we have parsed all the bytes. But in WAL splitting, we just return after getting EOF...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5016:
URL: https://github.com/apache/hbase/pull/5016#discussion_r1100233880
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/TagCompressionContext.java:
##########
@@ -107,15 +107,15 @@ public void uncompressTags(InputStream src, byte[] dest, int offset, int length)
throws IOException {
int endOffset = offset + length;
while (offset < endOffset) {
- byte status = (byte) src.read();
+ byte status = StreamUtils.readByte(src);
if (status == Dictionary.NOT_IN_DICTIONARY) {
int tagLen = StreamUtils.readRawVarint32(src);
offset = Bytes.putAsShort(dest, offset, tagLen);
IOUtils.readFully(src, dest, offset, tagLen);
- tagDict.addEntry(dest, offset, tagLen);
+ tagDict.findEntry(dest, offset, tagLen);
Review Comment:
Do you think this change could be expensive? In the normal case, the entry will not exist in the dict. But now we're adding an extra map lookup for every call. Granted o(1), but involves cpu for hashcode, allocating lookup key, etc.
I wonder if we could trigger findEntry only if context has been reset? Otherwise use addEntry for first pass?
May not be a big issue, just checking
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1423114977
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 22s | master passed |
| +1 :green_heart: | compile | 0m 57s | master passed |
| +1 :green_heart: | shadedjars | 4m 36s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 35s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 16s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 34s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 35s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 58s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 204m 47s | hbase-server in the patch passed. |
| | | 230m 42s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5016 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 685adc51353b 5.4.0-1094-aws #102~18.04.1-Ubuntu SMP Tue Jan 10 21:07:03 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 6a34aa8195 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/2/testReport/ |
| Max. process+thread count | 2707 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/2/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1423754768
The PR can not solve all the problem so I convert it to draft to avoid others may merge it accidentally.
Thanks all for help reviewing and testing, especially @thangTang for pointing out the problem.
Will change the title and provide a new PR soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1423367899
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 4m 39s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 56s | master passed |
| +1 :green_heart: | compile | 1m 6s | master passed |
| +1 :green_heart: | shadedjars | 4m 38s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 42s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 10s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 33s | the patch passed |
| +1 :green_heart: | compile | 1m 7s | the patch passed |
| +1 :green_heart: | javac | 1m 7s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 33s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 15s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 209m 12s | hbase-server in the patch passed. |
| | | 240m 56s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5016 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 6d7399d360d4 5.4.0-135-generic #152-Ubuntu SMP Wed Nov 23 20:19:22 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 6a34aa8195 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/3/testReport/ |
| Max. process+thread count | 2638 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/3/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 merged pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 merged PR #5016:
URL: https://github.com/apache/hbase/pull/5016
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1422797649
> This seems like an ingenious idea. But I want to confirm that due to the eviction mechanism of LRUMap, even if findEntry is used instead of addEntry, is there still a possibility of inconsistent read-write path behavior in theory?
The most important thing here is to read WAL entries in order, and not skip any entries. If these two rules are guaranteed, it is OK to restart as many times as you want. And I think for replication, we must follow these two rules otherwise there will be data loss...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] thangTang commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "thangTang (via GitHub)" <gi...@apache.org>.
thangTang commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1424234111
> I tried to refactoring a bit but the implementation of ProtobufLogReader is too complicated. I think we'd better abstract two types of WAL.Reader for reading WAL file. One is StreamingReader, which is used in most cases, for example, WAL splitting, WAL printing, etc, where we only need to read the file once and usually for closed WAL files. There is no need to support reset and seek. The other is TailingReader, which is used by Replication, where we need to support reset and seek, and also we need to tell the upper layer whether we need to reset the compress context when calling reseting. The logic will be more complicated as we need to consider the requirements for tailing a WAL file which is currently being written. The refactoring will be a bit big so I do not think we should apply it to branch-2.5 and branch-2.4. So let's apply the simple fix here and file another issue to implement the big refactoring.
>
> Thanks.
I understand that this is a complicated and dirty job, I am ashamed that I didn't solve it thoroughly before...
But by the way, just for this PR, Would you mind taking a look at https://issues.apache.org/jira/browse/HBASE-26850 and https://github.com/apache/hbase/pull/4233?
At that time, I thought that it could not fundamentally solve the problem, so I did not continue to push forward, but these two patches seem a bit similar? The difference is that I changed the implementation of addEntry.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] sunhelly commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "sunhelly (via GitHub)" <gi...@apache.org>.
sunhelly commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1426551447
It works well after disabling WAL value compress with this fix PR on our cluster. We can reproduce the replication stuck by enable WAL value compression, while the WALPrettyPrinter stops at the middle position without any exceptions. The stuck issue now is not relevant to the dictionary.
Great job! Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1425569416
> The stuck still occurs after diabling WAL value compression.
Is it OK for your company to upload the WAL file somewhere? So we can see the content of the WAL file and check what is the problem...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] sunhelly commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "sunhelly (via GitHub)" <gi...@apache.org>.
sunhelly commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1425530125
Oh, the cluster B really has lose data issue..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1424727540
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 24s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 31s | master passed |
| +1 :green_heart: | compile | 0m 56s | master passed |
| +1 :green_heart: | shadedjars | 4m 36s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 15s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 35s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 33s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 0s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 197m 53s | hbase-server in the patch passed. |
| | | 224m 25s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5016 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 8f57a4aad181 5.4.0-1093-aws #102~18.04.2-Ubuntu SMP Wed Dec 7 00:31:59 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / a854cba59f |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/4/testReport/ |
| Max. process+thread count | 2493 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/4/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a diff in pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #5016:
URL: https://github.com/apache/hbase/pull/5016#discussion_r1100301910
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/TagCompressionContext.java:
##########
@@ -107,15 +107,15 @@ public void uncompressTags(InputStream src, byte[] dest, int offset, int length)
throws IOException {
int endOffset = offset + length;
while (offset < endOffset) {
- byte status = (byte) src.read();
+ byte status = StreamUtils.readByte(src);
if (status == Dictionary.NOT_IN_DICTIONARY) {
int tagLen = StreamUtils.readRawVarint32(src);
offset = Bytes.putAsShort(dest, offset, tagLen);
IOUtils.readFully(src, dest, offset, tagLen);
- tagDict.addEntry(dest, offset, tagLen);
+ tagDict.findEntry(dest, offset, tagLen);
Review Comment:
It is slower than before but I always think correctness comes first, and then we consider the performance. For log splitting and replication, reading is usually not the bottleneck.
Can file an follow on issue to do the optimization, maybe we could add a reset flag in CompressionContext too, to indicate that whether we need to do a lookup first.
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] sunhelly commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "sunhelly (via GitHub)" <gi...@apache.org>.
sunhelly commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1423979453
For ensure the compress and uncompress construct same dictionary, we should only use LRUDictionary#findEntry() to add entries, but need to keep LRUDictionary#getEntry() do not move ahead the entry?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1425241509
@sunhelly Could you please try to see if this PR can also solve your problem?
And is it possible to contribute your replication test case to hbase-it?
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1425395308
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 58s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 36s | master passed |
| +1 :green_heart: | compile | 1m 6s | master passed |
| +1 :green_heart: | shadedjars | 4m 34s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 36s | the patch passed |
| +1 :green_heart: | compile | 1m 5s | the patch passed |
| +1 :green_heart: | javac | 1m 5s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 35s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 13s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 210m 12s | hbase-server in the patch passed. |
| | | 238m 2s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5016 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 144cedb192cc 5.4.0-135-generic #152-Ubuntu SMP Wed Nov 23 20:19:22 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / a854cba59f |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/5/testReport/ |
| Max. process+thread count | 2622 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/5/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1425469807
> I tested morning, sadly still something wrong...The problem is focus on one scenario, replicated mostly whole row deletes. It seems should not be relevant to the operation, but I can't find more relevant changes. We have already reset the compress context to fix the issue in the last months, it resolved most problems and seems more stable than before. But we have one circumstance, the replication always stuck. The senerio is as follows. There is two-way replications between cluster A and cluster B(both using wal group), A without WAL compression, B with WAL compression, write operations only on A. Now there are many whole row deletes on A, the replication of A->B is OK, the replication of B->A is always stucks, and the stuck is not rare, it is very easy to happen. I can not reproduce this problem locally until now, maybe it's not relevant to the uncompress progress, maybe something wrong when compress and the WAL is corrupt. I used WALPrettyPrinter to read these WALs, the printe
r always stopped at the same position for one WAL, and no exceptions output, but the end read position of the printer is in the middle of the WAL.
If WALPrettyPrinter can not output correct result, I think the problem is not about the replication implementation then, it should be something wrong when writing the WAL file. And I believe it will also make WAL splitting incorrect?
Do you also enabled WAL value compression? Or just the dictionary based compression...
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1422751647
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 19s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 25s | master passed |
| +1 :green_heart: | compile | 1m 10s | master passed |
| +1 :green_heart: | shadedjars | 5m 8s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 49s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 10s | the patch passed |
| +1 :green_heart: | compile | 1m 16s | the patch passed |
| +1 :green_heart: | javac | 1m 16s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 15s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 46s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 19s | hbase-common in the patch passed. |
| -1 :x: | unit | 215m 9s | hbase-server in the patch failed. |
| | | 247m 1s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5016 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 099dc80b3993 5.4.0-137-generic #154-Ubuntu SMP Thu Jan 5 17:03:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1a9e465e35 |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/1/testReport/ |
| Max. process+thread count | 2338 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/1/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1422487947
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 21s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 12s | master passed |
| +1 :green_heart: | compile | 3m 33s | master passed |
| +1 :green_heart: | checkstyle | 0m 55s | master passed |
| +1 :green_heart: | spotless | 0m 46s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 2m 54s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 10s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 34s | the patch passed |
| +1 :green_heart: | compile | 3m 19s | the patch passed |
| +1 :green_heart: | javac | 3m 19s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 15s | The patch passed checkstyle in hbase-common |
| +1 :green_heart: | checkstyle | 0m 36s | hbase-server: The patch generated 0 new + 5 unchanged - 2 fixed = 5 total (was 7) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 20m 14s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 57s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 3m 10s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 20s | The patch does not generate ASF License warnings. |
| | | 59m 6s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5016 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 0655804e6aa8 5.4.0-137-generic #154-Ubuntu SMP Thu Jan 5 17:03:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1a9e465e35 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 84 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/1/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5016:
URL: https://github.com/apache/hbase/pull/5016#discussion_r1100342132
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/TagCompressionContext.java:
##########
@@ -107,15 +107,15 @@ public void uncompressTags(InputStream src, byte[] dest, int offset, int length)
throws IOException {
int endOffset = offset + length;
while (offset < endOffset) {
- byte status = (byte) src.read();
+ byte status = StreamUtils.readByte(src);
if (status == Dictionary.NOT_IN_DICTIONARY) {
int tagLen = StreamUtils.readRawVarint32(src);
offset = Bytes.putAsShort(dest, offset, tagLen);
IOUtils.readFully(src, dest, offset, tagLen);
- tagDict.addEntry(dest, offset, tagLen);
+ tagDict.findEntry(dest, offset, tagLen);
Review Comment:
Sounds good, agree on correctness first.
Also agree on bottleneck for splitting/replications. However, this uncompressTags method is in the hot path of normal reads when DataBlockEncoding is used: [here](https://github.com/apache/hbase/blob/master/hbase-common/src/main/java/org/apache/hadoop/hbase/io/encoding/BufferedDataBlockEncoder.java#L828).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1425153110
> > I tried to refactoring a bit but the implementation of ProtobufLogReader is too complicated. I think we'd better abstract two types of WAL.Reader for reading WAL file. One is StreamingReader, which is used in most cases, for example, WAL splitting, WAL printing, etc, where we only need to read the file once and usually for closed WAL files. There is no need to support reset and seek. The other is TailingReader, which is used by Replication, where we need to support reset and seek, and also we need to tell the upper layer whether we need to reset the compress context when calling reseting. The logic will be more complicated as we need to consider the requirements for tailing a WAL file which is currently being written. The refactoring will be a bit big so I do not think we should apply it to branch-2.5 and branch-2.4. So let's apply the simple fix here and file another issue to implement the big refactoring.
> > Thanks.
>
> I understand that this is a complicated and dirty job, I am ashamed that I didn't solve it thoroughly before... But by the way, just for this PR, Would you mind taking a look at https://issues.apache.org/jira/browse/HBASE-26850 and #4233? At that time, I thought that it could not fundamentally solve the problem, so I did not continue to push forward, but these two patches seem a bit similar? The difference is that I changed the implementation of addEntry.
I think this can be done step by step.
First, we apply the patch here to fix the problem first, where the performance maybe bad than before. And then, we refactor the Reader, to introduce two types of Reader, so we can focus on how to improve the performance of tailing the WAL file which is being written currently in Repliaction without affecting the WAL splitting logic. Then we could try to introduce fine-grained control on whether we should reconstruct the dictionary, and finally, we could try to improve the LRUDictionary to support checkpoint and rollback, and do a checkpoint at a proper place and use rollback instead of clear and reconstruct, to get all the performance back.
WDYT?
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1425175909
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 23s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 18s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 15s | master passed |
| +1 :green_heart: | compile | 2m 52s | master passed |
| +1 :green_heart: | checkstyle | 0m 43s | master passed |
| +1 :green_heart: | spotless | 0m 37s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 46s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 10s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 20s | the patch passed |
| +1 :green_heart: | compile | 3m 0s | the patch passed |
| +1 :green_heart: | javac | 3m 0s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 12s | The patch passed checkstyle in hbase-common |
| +1 :green_heart: | checkstyle | 0m 31s | hbase-server: The patch generated 0 new + 9 unchanged - 2 fixed = 9 total (was 11) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 45s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 37s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 2m 3s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 14s | The patch does not generate ASF License warnings. |
| | | 40m 21s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5016 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux e86193e8d189 5.4.0-1094-aws #102~18.04.1-Ubuntu SMP Tue Jan 10 21:07:03 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / a854cba59f |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/5/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1422861397
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 25s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 30s | master passed |
| +1 :green_heart: | compile | 2m 55s | master passed |
| +1 :green_heart: | checkstyle | 0m 43s | master passed |
| +1 :green_heart: | spotless | 0m 40s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 56s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 9s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 13s | the patch passed |
| +1 :green_heart: | compile | 2m 51s | the patch passed |
| +1 :green_heart: | javac | 2m 51s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 12s | The patch passed checkstyle in hbase-common |
| +1 :green_heart: | checkstyle | 0m 31s | hbase-server: The patch generated 0 new + 5 unchanged - 2 fixed = 5 total (was 7) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 43s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 38s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 2m 3s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
| | | 40m 45s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5016 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux af035ff80124 5.4.0-1094-aws #102~18.04.1-Ubuntu SMP Tue Jan 10 21:07:03 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 6a34aa8195 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/2/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1423552347
> > > This seems like an ingenious idea. But I want to confirm that due to the eviction mechanism of LRUMap, even if findEntry is used instead of addEntry, is there still a possibility of inconsistent read-write path behavior in theory?
> >
> >
> > The most important thing here is to read WAL entries in order, and not skip any entries. If these two rules are guaranteed, it is OK to restart as many times as you want. And I think for replication, we must follow these two rules otherwise there will be data loss...
>
> Agree about that, but I think I didn't express my question clearly.
>
> For WAL Compression, The core logic is to build an index (LRUMap) in memory while writing/reading WAL. There is another key point here, that is, when operating a WAL file, the behavior of both read/write path needs to be exactly same.
>
> Using findEntry instead of addEntry in this patch, I think it could solve a part of problem. But however, for example, we did not resetPosition when we wrote WAL, but a certain position was reset many times when we read WAL. The implicit operation here is: this node has been movedToHead many times in LRUMap. So is it possible that the node evicted in the write path(write WAL) has inconsistencies in the read path(replication)?
After deep consideration I think you are right. The solution here can only work perfectly when the dict is infinite, i.e, no eviction. If we also consider eviction, if go back for a long distance, the word of a given index will change due to eviction, then when reading, if we use a index to get the word(a qualifier, a row, for example), we may get a incorrect word on the given index.
In real world, although it is not likely that we will go back for a very long distance, but it is possible that a single WAL entry has a lot of cells, and our qualifier dict capacity is only 127, it is still possible to fall into the above scenario...
So, it seems that rebuilding the dict is necessary when reseting. But anyway, I could try to refactor the readNext method in ProtrobufLogReader, to have more fine-grained control on whether we need to reconstruct the dict. For example, if we just return before reading the actual WAL entry, i.e, we quit earlier after checking available bytes, we do not need to reconstruct the dict.
Thanks for pointing this out!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1422730625
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 3m 5s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 16s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 7s | master passed |
| +1 :green_heart: | compile | 1m 23s | master passed |
| +1 :green_heart: | shadedjars | 5m 25s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 47s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 44s | the patch passed |
| +1 :green_heart: | compile | 0m 55s | the patch passed |
| +1 :green_heart: | javac | 0m 55s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 37s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 34s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 1s | hbase-common in the patch passed. |
| -1 :x: | unit | 202m 55s | hbase-server in the patch failed. |
| | | 234m 28s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5016 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 89a079cfa76f 5.4.0-1094-aws #102~18.04.1-Ubuntu SMP Tue Jan 10 21:07:03 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1a9e465e35 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/1/testReport/ |
| Max. process+thread count | 2668 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/1/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] thangTang commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "thangTang (via GitHub)" <gi...@apache.org>.
thangTang commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1423582324
> So, it seems that rebuilding the dict is necessary when reseting.
Agree. Although this solution will have a performance loss, but it should be the best way I can think of to completely solve this problem.
Another idea is to refactor dict and design an LRUMap that can support precise rollback. I've spent some time in this direction, but found nothing out. At least, it also can't be free (such as memory overhead). . .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] thangTang commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "thangTang (via GitHub)" <gi...@apache.org>.
thangTang commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1424240190
@apurtell FYI, I think you may also be interested in this patch~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] thangTang commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "thangTang (via GitHub)" <gi...@apache.org>.
thangTang commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1424241337
After all, manual +1 from me: )
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1425391313
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 48s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 48s | master passed |
| +1 :green_heart: | compile | 0m 56s | master passed |
| +1 :green_heart: | shadedjars | 4m 15s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 46s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 18s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 45s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 211m 35s | hbase-server in the patch passed. |
| | | 236m 15s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5016 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux cd3608f7d3ac 5.4.0-137-generic #154-Ubuntu SMP Thu Jan 5 17:03:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / a854cba59f |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/5/testReport/ |
| Max. process+thread count | 2345 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/5/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1424191961
I tried to refactoring a bit but the implementation of ProtobufLogReader is too complicated. I think we'd better abstract two types of WAL.Reader for reading WAL file.
One is StreamingReader, which is used in most cases, for example, WAL splitting, WAL printing, etc, where we only need to read the file once and usually for closed WAL files. There is no need to support reset and seek.
The other is TailingReader, which is used by Replication, where we need to support reset and seek, and also we need to tell the upper layer whether we need to reset the compress context when calling reseting. The logic will be more complicated as we need to consider the requirements for tailing a WAL file which is currently being written.
The refactoring will be a bit big so I do not think we should apply it to branch-2.5 and branch-2.4. So let's apply the simple fix here and file another issue to implement the big refactoring.
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] thangTang commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "thangTang (via GitHub)" <gi...@apache.org>.
thangTang commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1422437691
This seems like an ingenious idea. But I want to confirm that due to the eviction mechanism of LRUMap, even if findEntry is used instead of addEntry, is there still a possibility of inconsistent read-write path behavior in theory?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1423119488
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 49s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 45s | master passed |
| +1 :green_heart: | compile | 0m 56s | master passed |
| +1 :green_heart: | shadedjars | 4m 15s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 49s | the patch passed |
| +1 :green_heart: | compile | 0m 58s | the patch passed |
| +1 :green_heart: | javac | 0m 58s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 17s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 48s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 209m 56s | hbase-server in the patch passed. |
| | | 234m 51s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5016 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 8b13800be3c3 5.4.0-137-generic #154-Ubuntu SMP Thu Jan 5 17:03:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 6a34aa8195 |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/2/testReport/ |
| Max. process+thread count | 2387 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/2/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1424184792
> For ensure the compress and uncompress construct same dictionary, we should only use LRUDictionary#findEntry() to add entries, but need to keep LRUDictionary#getEntry() do not move ahead the entry?
This is still not enough... As said above, if we go back for a long distance, the word on a given index could be completely different, and then lead to incorrect result when you find a field is 'in dictionary'...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] sunhelly commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "sunhelly (via GitHub)" <gi...@apache.org>.
sunhelly commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1425433196
I tested morning, sadly still something wrong...The problem is focus on one scenario, replicated mostly whole row deletes. It seems should not be relevant to the operation, but I can't find more relevant changes.
We have already reset the compress context to fix the issue in the last months, it resolved most problems and seems more stable than before. But we have one circumstance, the replication always stuck. The senerio is as follows.
There is two-way replications between cluster A and cluster B(both using wal group), A without WAL compression, B with WAL compression, write operations only on A. Now there are many whole row deletes on A, the replication of A->B is OK, the replication of B->A is always stucks, and the stuck is not rare, it is very easy to happen.
I can not reproduce this problem locally until now, maybe it's not relevant to the uncompress progress, maybe something wrong when compress and the WAL is corrupt. I used WALPrettyPrinter to read these WALs, the printer always stopped at the same position for one WAL, and no exceptions output, but the end read position of the printer is in the middle of the WAL.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] sunhelly commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "sunhelly (via GitHub)" <gi...@apache.org>.
sunhelly commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1425567577
The stuck still occurs after diabling WAL value compression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] sunhelly commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "sunhelly (via GitHub)" <gi...@apache.org>.
sunhelly commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1425606429
OK. I'll prepare one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1426705422
Thanks @sunhelly for providing the useful feedback.
Let me merge this PR first to solve the dictionary problem.
For replication value compression, seems there are still other bugs and @apurtell also pointed out that there are some tricks in the buffer reuse mechanism, will dig more and file other issues to try to fix.
Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] sunhelly commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "sunhelly (via GitHub)" <gi...@apache.org>.
sunhelly commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1424032823
Tested to remove moveToHead() at line https://github.com/apache/hbase/blob/master/hbase-common/src/main/java/org/apache/hadoop/hbase/io/util/LRUDictionary.java#L154, our cluster replication with WAL compression works well now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5016:
URL: https://github.com/apache/hbase/pull/5016#discussion_r1100342132
##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/TagCompressionContext.java:
##########
@@ -107,15 +107,15 @@ public void uncompressTags(InputStream src, byte[] dest, int offset, int length)
throws IOException {
int endOffset = offset + length;
while (offset < endOffset) {
- byte status = (byte) src.read();
+ byte status = StreamUtils.readByte(src);
if (status == Dictionary.NOT_IN_DICTIONARY) {
int tagLen = StreamUtils.readRawVarint32(src);
offset = Bytes.putAsShort(dest, offset, tagLen);
IOUtils.readFully(src, dest, offset, tagLen);
- tagDict.addEntry(dest, offset, tagLen);
+ tagDict.findEntry(dest, offset, tagLen);
Review Comment:
Sounds good, agree on correctness first.
Also agree on bottleneck for splitting/replications. However, this uncompressTags method is in the hot path of normal reads when DataBlockEncoding is used.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1423371925
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 14s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 37s | master passed |
| +1 :green_heart: | compile | 1m 7s | master passed |
| +1 :green_heart: | shadedjars | 5m 13s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 49s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 11s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 9s | the patch passed |
| +1 :green_heart: | compile | 1m 14s | the patch passed |
| +1 :green_heart: | javac | 1m 14s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 17s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 2m 17s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 214m 48s | hbase-server in the patch passed. |
| | | 246m 17s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5016 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 6a2964fb26a9 5.4.0-137-generic #154-Ubuntu SMP Thu Jan 5 17:03:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 6a34aa8195 |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/3/testReport/ |
| Max. process+thread count | 2462 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/3/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1424740826
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 46s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 45s | master passed |
| +1 :green_heart: | compile | 0m 55s | master passed |
| +1 :green_heart: | shadedjars | 4m 16s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 53s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 15s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 45s | hbase-common in the patch passed. |
| +1 :green_heart: | unit | 211m 19s | hbase-server in the patch passed. |
| | | 235m 55s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5016 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 32aa9c6efaca 5.4.0-137-generic #154-Ubuntu SMP Thu Jan 5 17:03:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / a854cba59f |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/4/testReport/ |
| Max. process+thread count | 2250 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/4/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] thangTang commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "thangTang (via GitHub)" <gi...@apache.org>.
thangTang commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1425170220
> > > I tried to refactoring a bit but the implementation of ProtobufLogReader is too complicated. I think we'd better abstract two types of WAL.Reader for reading WAL file. One is StreamingReader, which is used in most cases, for example, WAL splitting, WAL printing, etc, where we only need to read the file once and usually for closed WAL files. There is no need to support reset and seek. The other is TailingReader, which is used by Replication, where we need to support reset and seek, and also we need to tell the upper layer whether we need to reset the compress context when calling reseting. The logic will be more complicated as we need to consider the requirements for tailing a WAL file which is currently being written. The refactoring will be a bit big so I do not think we should apply it to branch-2.5 and branch-2.4. So let's apply the simple fix here and file another issue to implement the big refactoring.
> > > Thanks.
> >
> >
> > I understand that this is a complicated and dirty job, I am ashamed that I didn't solve it thoroughly before... But by the way, just for this PR, Would you mind taking a look at https://issues.apache.org/jira/browse/HBASE-26850 and #4233? At that time, I thought that it could not fundamentally solve the problem, so I did not continue to push forward, but these two patches seem a bit similar? The difference is that I changed the implementation of addEntry.
>
> I think this can be done step by step. First, we apply the patch here to fix the problem first, where the performance maybe bad than before. And then, we refactor the Reader, to introduce two types of Reader, so we can focus on how to improve the performance of tailing the WAL file which is being written currently in Repliaction without affecting the WAL splitting logic. Then we could try to introduce fine-grained control on whether we should reconstruct the dictionary, and finally, we could try to improve the LRUDictionary to support checkpoint and rollback, and do a checkpoint at a proper place and use rollback instead of clear and reconstruct, to get all the performance back.
>
> WDYT?
>
> Thanks.
Make sense.
+1 from me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] sunhelly commented on pull request #5016: HBASE-27621 Also clear the Dictionary when resetting when reading compressed WAL file
Posted by "sunhelly (via GitHub)" <gi...@apache.org>.
sunhelly commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1425492451
Yes, I also enabled WAL value compression. I'll check if the stuck recurs after disable it.
And there are no WAL splitting issues until now. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] thangTang commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "thangTang (via GitHub)" <gi...@apache.org>.
thangTang commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1422840125
> > This seems like an ingenious idea. But I want to confirm that due to the eviction mechanism of LRUMap, even if findEntry is used instead of addEntry, is there still a possibility of inconsistent read-write path behavior in theory?
>
> The most important thing here is to read WAL entries in order, and not skip any entries. If these two rules are guaranteed, it is OK to restart as many times as you want. And I think for replication, we must follow these two rules otherwise there will be data loss...
Agree about that, but I think I didn't express my question clearly.
For WAL Compression, The core logic is to build an index (LRUMap) in memory while writing/reading WAL. There is another key point here, that is, when operating a WAL file, the behavior of the read-write path needs to be exactly same.
Using findEntry instead of addEntry in this patch, I think it could solve a part of problem. But however, for example, we did not resetPosition when we wrote WAL, but a certain position was reset many times when we read WAL. The implicit operation here is: this node has been movedToHead many times in LRUMap. So is it possible that the node evicted in the write path(write WAL) has inconsistencies in the read path(replication)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #5016: HBASE-27621 Always use findEntry to fill the Dictionary when reading …
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1423189170
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 12s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 19s | master passed |
| +1 :green_heart: | compile | 3m 41s | master passed |
| +1 :green_heart: | checkstyle | 0m 58s | master passed |
| +1 :green_heart: | spotless | 0m 46s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 2m 55s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 9s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 41s | the patch passed |
| +1 :green_heart: | compile | 3m 17s | the patch passed |
| +1 :green_heart: | javac | 3m 17s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 13s | The patch passed checkstyle in hbase-common |
| +1 :green_heart: | checkstyle | 0m 43s | hbase-server: The patch generated 0 new + 5 unchanged - 2 fixed = 5 total (was 7) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 19m 58s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 55s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 3m 11s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 21s | The patch does not generate ASF License warnings. |
| | | 58m 38s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.42 ServerAPI=1.42 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5016 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 3f1e83ca043a 5.4.0-137-generic #154-Ubuntu SMP Thu Jan 5 17:03:22 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 6a34aa8195 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-common hbase-server U: . |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5016/3/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org