Skip to content

HBASE-25541: Setting the path to null when we dequeue the current log - #2959

Merged
virajjasani merged 1 commit into
apache:masterfrom
sandeepvinayak:HBASE-25541
Feb 18, 2021
Merged

HBASE-25541: Setting the path to null when we dequeue the current log#2959
virajjasani merged 1 commit into
apache:masterfrom
sandeepvinayak:HBASE-25541

Conversation

@sandeepvinayak

@sandeepvinayaksandeepvinayak commented Feb 16, 2021

Copy link
Copy Markdown
Contributor

Currently, when we deque we do not reset the current wal path, and it blows up while opening the next log, the current path still points to previous wal which is not correct.

privatebooleanopenNextLog() throwsIOException {
PathnextPath = logQueue.peek();
if (nextPath != null) {
openReader(nextPath); <=== ifweblowuphereif (reader != null) {
returntrue;
}
} else {
// no more files in queue, this could happen for recovered queue, or for a wal group of a sync// replication peer which has already been transited to DA or S.setCurrentPath(null);
}
returnfalse;
} 

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 20sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
_ master Compile Tests _
+1 💚mvninstall3m 51smaster passed
+1 💚compile3m 11smaster passed
+1 💚checkstyle1m 4smaster passed
+1 💚spotbugs2m 3smaster passed
_ Patch Compile Tests _
+1 💚mvninstall3m 29sthe patch passed
+1 💚compile3m 9sthe patch passed
+1 💚javac3m 9sthe patch passed
+1 💚checkstyle1m 2sthe patch passed
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚hadoopcheck17m 13sPatch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.
+1 💚spotbugs2m 7sthe patch passed
_ Other Tests _
+1 💚asflicense0m 15sThe patch does not generate ASF License warnings.
46m 17s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2959/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#2959
JIRA IssueHBASE-25541
Optional Testsdupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile
unameLinux b7d96ffb323d 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / b6649a8
Default JavaAdoptOpenJDK-1.8.0_232-b09
Max. process+thread count95 (vs. ulimit of 30000)
modulesC: hbase-server U: hbase-server
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2959/1/console
versionsgit=2.17.1 maven=3.6.3 spotbugs=3.1.12
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@bharathvbharathv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nit to add some detail to the commit message to set the context.

LOG.debug("EOF, closing {}", currentPath);
closeReader();
logQueue.remove();
setCurrentPath(null);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix, can you add some detail to the commit message, I think it's subtle.

Looks like the issue is if openNextLog() throws, we want currentPath to be null rather than stale.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have good context here, but is it possible to add a small test? That could help anyone get to know this bit better, it's fine though if it is too complicated to add it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got to know from @sandeepvinayak that writing test is complicated. We can skip it.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec3m 47sDocker mode activated.
-0 ⚠️yetus0m 4sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚mvninstall3m 51smaster passed
+1 💚compile0m 57smaster passed
+1 💚shadedjars6m 38sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 38smaster passed
_ Patch Compile Tests _
+1 💚mvninstall3m 23sthe patch passed
+1 💚compile0m 58sthe patch passed
+1 💚javac0m 58sthe patch passed
+1 💚shadedjars6m 38spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 36sthe patch passed
_ Other Tests _
+1 💚unit142m 24shbase-server in the patch passed.
172m 13s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2959/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR#2959
JIRA IssueHBASE-25541
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 160676e5f9dd 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / b6649a8
Default JavaAdoptOpenJDK-1.8.0_232-b09
Test Resultshttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2959/1/testReport/
Max. process+thread count4700 (vs. ulimit of 30000)
modulesC: hbase-server U: hbase-server
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2959/1/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 12sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+1 💚mvninstall4m 54smaster passed
+1 💚compile1m 15smaster passed
+1 💚shadedjars7m 29sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 43smaster passed
_ Patch Compile Tests _
+1 💚mvninstall4m 39sthe patch passed
+1 💚compile1m 14sthe patch passed
+1 💚javac1m 14sthe patch passed
+1 💚shadedjars7m 28spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 43sthe patch passed
_ Other Tests _
+1 💚unit199m 17shbase-server in the patch passed.
230m 46s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2959/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#2959
JIRA IssueHBASE-25541
Optional Testsjavac javadoc unit shadedjars compile
unameLinux a7f3885b30f4 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / b6649a8
Default JavaAdoptOpenJDK-11.0.6+10
Test Resultshttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2959/1/testReport/
Max. process+thread count3265 (vs. ulimit of 30000)
modulesC: hbase-server U: hbase-server
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2959/1/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@virajjasani
virajjasani merged commit 1beda0d into apache:masterFeb 18, 2021
virajjasani pushed a commit that referenced this pull request Feb 18, 2021
#2959)
Signed-off-by: Bharath Vissapragada <bharathv@apache.org>
virajjasani pushed a commit that referenced this pull request Feb 18, 2021
#2959)
Signed-off-by: Bharath Vissapragada <bharathv@apache.org>
virajjasani pushed a commit that referenced this pull request Feb 18, 2021
#2959)
Signed-off-by: Bharath Vissapragada <bharathv@apache.org>
virajjasani pushed a commit that referenced this pull request Feb 18, 2021
#2959)
Signed-off-by: Bharath Vissapragada <bharathv@apache.org>
virajjasani pushed a commit that referenced this pull request Feb 18, 2021
#2959)
Signed-off-by: Bharath Vissapragada <bharathv@apache.org>
ddupg pushed a commit to ddupg/hbase that referenced this pull request Feb 19, 2021
apache#2959)
Signed-off-by: Bharath Vissapragada <bharathv@apache.org>
wchevreuil pushed a commit to wchevreuil/hbase that referenced this pull request Feb 24, 2021
apache#2959)
Signed-off-by: Bharath Vissapragada <bharathv@apache.org>
wchevreuil pushed a commit to wchevreuil/hbase that referenced this pull request May 24, 2021
apache#2959)
Signed-off-by: Bharath Vissapragada <bharathv@apache.org>
(cherry picked from commit ed9f8e6)
Change-Id: I6d68df2f91a53fc8eae1a75bc63d847e0da2d63b
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@sandeepvinayak@Apache-HBase@bharathv@virajjasani