Skip to content

HDFS-16709. Remove redundant cast in FSEditLogOp.class - #4667

Merged
ayushtkn merged 1 commit into
apache:trunkfrom
ZanderXu:HDFS-16709
Aug 8, 2022
Merged

HDFS-16709. Remove redundant cast in FSEditLogOp.class#4667
ayushtkn merged 1 commit into
apache:trunkfrom
ZanderXu:HDFS-16709

Conversation

@ZanderXu

Copy link
Copy Markdown
Contributor

Description of PR

When I read some class about Edits of NameNode, I found that there are much redundant cast in FSEditLogOp.class, I feel that we should remove them.

Such as:

static UpdateBlocksOp getInstance(OpInstanceCache cache) {
return (UpdateBlocksOp)cache.get(OP_UPDATE_BLOCKS);
}

Because cache.get() have cast the response to T, so we can remove the redundant cast.

@SuppressWarnings("unchecked")
public <T extends FSEditLogOp> T get(FSEditLogOpCodes opCode) {
return useCache ? (T)CACHE.get().get(opCode) : (T)newInstance(opCode);
} 

@slfan1989

Copy link
Copy Markdown
Contributor

Can the unchecked flag be removed?

@SuppressWarnings("unchecked")
public <T extends FSEditLogOp> T get(FSEditLogOpCodes opCode) {
return useCache ? (T)CACHE.get().get(opCode) : (T)newInstance(opCode);
} 

@hadoop-yetus

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 54sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 1sNo case conflicting files found.
+0 🆗codespell0m 0scodespell was not available.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
+1 💚test4tests0m 0sThe patch appears to include 1 new or modified test files.
_ trunk Compile Tests _
+1 💚mvninstall38m 30strunk passed
+1 💚compile1m 45strunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚compile1m 36strunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚checkstyle1m 27strunk passed
+1 💚mvnsite1m 50strunk passed
+1 💚javadoc1m 25strunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚javadoc1m 47strunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚spotbugs3m 43strunk passed
+1 💚shadedclient23m 21sbranch has no errors when building and testing our client artifacts.
_ Patch Compile Tests _
+1 💚mvninstall1m 24sthe patch passed
+1 💚compile1m 26sthe patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚javac1m 26sthe patch passed
+1 💚compile1m 19sthe patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚javac1m 19sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
+1 💚checkstyle1m 1sthe patch passed
+1 💚mvnsite1m 26sthe patch passed
+1 💚javadoc0m 58sthe patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚javadoc1m 32sthe patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚spotbugs3m 23sthe patch passed
+1 💚shadedclient22m 46spatch has no errors when building and testing our client artifacts.
_ Other Tests _
-1 ❌unit416m 26s/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txthadoop-hdfs in the patch passed.
+1 💚asflicense1m 17sThe patch does not generate ASF License warnings.
527m 36s
ReasonTests
Failed junit testshadoop.hdfs.server.blockmanagement.TestBlockTokenWithShortCircuitRead
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4667/1/artifact/out/Dockerfile
GITHUB PR#4667
Optional Testsdupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
unameLinux be082fe1e5d1 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/bin/hadoop.sh
git revisiontrunk / 6154108
Default JavaPrivate Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions/usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Test Resultshttps://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4667/1/testReport/
Max. process+thread count3386 (vs. ulimit of 5500)
modulesC: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
Console outputhttps://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4667/1/console
versionsgit=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered byApache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

@ZanderXu

Copy link
Copy Markdown
ContributorAuthor

Thanks @slfan1989 for your review.

Can the unchecked flag be removed?

I think we need it. Because after removing it, the IDEA warns Unchecked cast:org.apache.hadoop.hdfs.server.namenode.FSEditLogOp to T.

@ayushtknayushtkn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@slfan1989

Copy link
Copy Markdown
Contributor

@ZanderXu

I think we need it. Because after removing it, the IDEA warns Unchecked cast:org.apache.hadoop.hdfs.server.namenode.FSEditLogOp to T.

Thanks for the explanation, I have understood your changes.
LGTM.

@HexiaoqiaoHexiaoqiao 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.

LGTM. +1.

@ZanderXu

Copy link
Copy Markdown
ContributorAuthor

@Hexiaoqiao@ayushtkn@goiri Master, can help me merge this PR into trunk? Thanks

@ayushtkn
ayushtkn merged commit 895f7c5 into apache:trunkAug 8, 2022
@ZanderXu

Copy link
Copy Markdown
ContributorAuthor

@slfan1989@Hexiaoqiao@goiri@ayushtkn Master, thank you very much for helping me review this patch.

HarshitGupta11 pushed a commit to HarshitGupta11/hadoop that referenced this pull request Nov 28, 2022
… Contributed by ZanderXu.
Reviewed-by: Inigo Goiri <inigoiri@apache.org>
Reviewed-by: He Xiaoqiao <hexiaoqiao@apache.org>
Signed-off-by: Ayush Saxena <ayushsaxena@apache.org>
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.

6 participants

@ZanderXu@slfan1989@hadoop-yetus@Hexiaoqiao@goiri@ayushtkn