HADOOP-18222. Prevent DelegationTokenSecretManagerMetrics from registering multiple times #4266

hchaverri · 2022-05-05T19:43:34Z

Description of PR

How was this patch tested?

For code changes:

Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

…ering multiple times

omalley · 2022-05-05T23:45:35Z

The tests fail, because now the static means the metrics are reused between tests.

  <testcase name="testDelegationTokenSecretManagerMetrics" classname="org.apache.hadoop.security.tok
en.delegation.TestDelegationToken" time="0.056">
    <failure message="expected:&lt;0&gt; but was:&lt;1&gt;" type="java.lang.AssertionError"><![CDATA[java.lang.AssertionError: expected:<0> but was:<1>
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.failNotEquals(Assert.java:835)
        at org.junit.Assert.assertEquals(Assert.java:647)
        at org.junit.Assert.assertEquals(Assert.java:633)
        at org.apache.hadoop.security.token.delegation.TestDelegationToken.callAndValidateMetrics(TestDelegationToken.java:711)
        at org.apache.hadoop.security.token.delegation.TestDelegationToken.testDelegationTokenSecretManagerMetrics(TestDelegationToken.java:664)

hchaverri · 2022-05-06T00:42:13Z

Updated tests to only check incremental changes in the counters and stats, instead of absolute values

hadoop-yetus · 2022-05-06T04:45:57Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 56s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s		The patch appears to include 1 new or modified test files.
			_ trunk Compile Tests _
+1 💚	mvninstall	41m 27s		trunk passed
+1 💚	compile	27m 50s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	24m 56s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	1m 33s		trunk passed
+1 💚	mvnsite	2m 10s		trunk passed
-1 ❌	javadoc	1m 40s	/branch-javadoc-hadoop-common-project_hadoop-common-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt	hadoop-common in trunk failed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.
+1 💚	javadoc	2m 4s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	3m 29s		trunk passed
+1 💚	shadedclient	26m 44s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	1m 5s		the patch passed
+1 💚	compile	25m 55s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javac	25m 55s		the patch passed
+1 💚	compile	23m 31s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	23m 31s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	1m 39s		the patch passed
+1 💚	mvnsite	2m 9s		the patch passed
-1 ❌	javadoc	1m 36s	/patch-javadoc-hadoop-common-project_hadoop-common-jdkPrivateBuild-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.txt	hadoop-common in the patch failed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1.
+1 💚	javadoc	2m 11s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	3m 15s		the patch passed
+1 💚	shadedclient	28m 8s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	19m 31s		hadoop-common in the patch passed.
+1 💚	asflicense	1m 19s		The patch does not generate ASF License warnings.
		243m 9s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4266/2/artifact/out/Dockerfile
GITHUB PR	#4266
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell
uname	Linux 11a5a6b27158 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `423f78a`
Default Java	Private 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 Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4266/2/testReport/
Max. process+thread count	3014 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4266/2/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

slfan1989 · 2022-05-07T02:30:16Z

Hi，@hchaverri，@omalley I'm very sorry, but I don't know if you can roll back the changes of HADOOP-18167 first. This change has caused many Junit Tests related to RM to fail. I tried to merge your changes in HADOOP-18222 locally, but Unit Test still reported an error.

Caused by: org.apache.hadoop.metrics2.MetricsException: Metrics source DelegationTokenSecretManagerMetrics already exists!
	at org.apache.hadoop.metrics2.lib.DefaultMetricsSystem.newSourceName(DefaultMetricsSystem.java:152)
	at org.apache.hadoop.metrics2.lib.DefaultMetricsSystem.sourceName(DefaultMetricsSystem.java:125)
	at org.apache.hadoop.metrics2.impl.MetricsSystemImpl.register(MetricsSystemImpl.java:229)
	at org.apache.hadoop.metrics2.MetricsSystem.register(MetricsSystem.java:71)
	at org.apache.hadoop.security.token.delegation.AbstractDelegationTokenSecretManager$DelegationTokenSecretManagerMetrics.create(AbstractDelegationTokenSecretManager.java:878)
	at org.apache.hadoop.security.token.delegation.AbstractDelegationTokenSecretManager.<init>(AbstractDelegationTokenSecretManager.java:152)
	at org.apache.hadoop.security.token.delegation.web.DelegationTokenManager$DelegationTokenSecretManager.<init>(DelegationTokenManager.java:72)
	at org.apache.hadoop.security.token.delegation.web.DelegationTokenManager.<init>(DelegationTokenManager.java:122)
	at org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticationHandler.initTokenManager(DelegationTokenAuthenticationHandler.java:161)
	at org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticationHandler.init(DelegationTokenAuthenticationHandler.java:130)
	at org.apache.hadoop.security.authentication.server.AuthenticationFilter.initializeAuthHandler(AuthenticationFilter.java:194)
	at org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticationFilter.initializeAuthHandler(DelegationTokenAuthenticationFilter.java:214)
	at org.apache.hadoop.security.authentication.server.AuthenticationFilter.init(AuthenticationFilter.java:180)
	at org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticationFilter.init(DelegationTokenAuthenticationFilter.java:180)
	at org.apache.hadoop.yarn.server.security.http.RMAuthenticationFilter.init(RMAuthenticationFilter.java:53)
	at org.eclipse.jetty.servlet.FilterHolder.initialize(FilterHolder.java:140)
	at org.eclipse.jetty.servlet.ServletHandler.lambda$initialize$0(ServletHandler.java:731)

szilard-nemeth · 2022-05-08T20:57:46Z

Hi @hchaverri ,
CC: @slfan1989
If this still causes test failures I'd also vote for reverting HADOOP-18167 first.
For example, I can't merge my changes in YARN-11114 as I don't have confidence if my changes have broken the RMWebServices tests or they are still caused by the double metrics registration issue originally caused by HADOOP-18167.
This is the main reason I reported YARN-11123.

slfan1989 · 2022-05-09T10:55:27Z

Hi @hchaverri ,
CC: @szilard-nemeth
My local merge hchaverri code has passed the test, and his solution is feasible. Change the DelegationTokenSecretManagerMetrics into a global declaration, which can guarantee 1 initialization.

I found out that he may have Blocking Java Doc compilation error, I try to solve it.

JIRA:https://issues.apache.org/jira/browse/HADOOP-18229
GitHub Pull Request #4292

szilard-nemeth · 2022-05-10T12:55:24Z

Hi @hchaverri , CC: @szilard-nemeth My local merge hchaverri code has passed the test, and his solution is feasible. Change the DelegationTokenSecretManagerMetrics into a global declaration, which can guarantee 1 initialization.

I found out that he may have Blocking Java Doc compilation error, I try to solve it.

JIRA:issues.apache.org/jira/browse/HADOOP-18229 GitHub Pull Request #4292

I see, thanks for the update.
How do you mean that Javadoc errors are "blocking"?
I don't see a relation between the javadoc issues and the DelegationTokenSecretManagerMetrics issue.
The latter causes unit tests to fail (maybe even production issues) so it should be fixed ASAP.
Thanks.

slfan1989 · 2022-05-10T14:08:59Z

Hi @hchaverri , CC: @szilard-nemeth My local merge hchaverri code has passed the test, and his solution is feasible. Change the DelegationTokenSecretManagerMetrics into a global declaration, which can guarantee 1 initialization.
I found out that he may have Blocking Java Doc compilation error, I try to solve it.
JIRA:issues.apache.org/jira/browse/HADOOP-18229 GitHub Pull Request #4292

I see, thanks for the update. How do you mean that Javadoc errors are "blocking"? I don't see a relation between the javadoc issues and the DelegationTokenSecretManagerMetrics issue. The latter causes unit tests to fail (maybe even production issues) so it should be fixed ASAP. Thanks.

There is no problem with your understanding. I thought this patch was not merged because of the Java Doc Error (hadoop-yetus -1), but I carefully read the compilation information, Java Doc Compile Error IN JDK11.

…ering multiple times Fixes #4266 Signed-off-by: Owen O'Malley <oomalley@linkedin.com>

…ering multiple times Fixes apache#4266 Signed-off-by: Owen O'Malley <oomalley@linkedin.com>

…ering multiple times Fixes apache#4266 Signed-off-by: Owen O'Malley <oomalley@linkedin.com> RB=3362276 BUG=LIHADOOP-65078 G=storage-reviewers R=oomalley,sdzinama A=sdzinama

hchaverri added 3 commits May 5, 2022 12:39

HADOOP-18222. Prevent DelegationTokenSecretManagerMetrics from regist…

7c1bcbe

…ering multiple times

Removing space

44eb0df

Removing space

cae27ad

hchaverri mentioned this pull request May 5, 2022

HADOOP-18167. Add metrics to track delegation token secret manager op… #4092

Merged

4 tasks

Updating test to only check relative counter changes

423f78a

slfan1989 mentioned this pull request May 10, 2022

YARN-11122. Support getClusterNodes API in FederationClientInterceptor #4274

Merged

omalley closed this in 99a83fd May 10, 2022

omalley pushed a commit that referenced this pull request May 10, 2022

HADOOP-18222. Prevent DelegationTokenSecretManagerMetrics from regist…

1e043b9

…ering multiple times Fixes #4266 Signed-off-by: Owen O'Malley <oomalley@linkedin.com>

HarshitGupta11 pushed a commit to HarshitGupta11/hadoop that referenced this pull request Nov 28, 2022

HADOOP-18222. Prevent DelegationTokenSecretManagerMetrics from regist…

5129ded

…ering multiple times Fixes apache#4266 Signed-off-by: Owen O'Malley <oomalley@linkedin.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HADOOP-18222. Prevent DelegationTokenSecretManagerMetrics from registering multiple times #4266

HADOOP-18222. Prevent DelegationTokenSecretManagerMetrics from registering multiple times #4266

hchaverri commented May 5, 2022

omalley commented May 5, 2022 •

edited

Loading

hchaverri commented May 6, 2022

hadoop-yetus commented May 6, 2022

slfan1989 commented May 7, 2022 •

edited

Loading

szilard-nemeth commented May 8, 2022

slfan1989 commented May 9, 2022 •

edited

Loading

szilard-nemeth commented May 10, 2022 •

edited

Loading

slfan1989 commented May 10, 2022 •

edited

Loading

HADOOP-18222. Prevent DelegationTokenSecretManagerMetrics from registering multiple times #4266

HADOOP-18222. Prevent DelegationTokenSecretManagerMetrics from registering multiple times #4266

Conversation

hchaverri commented May 5, 2022

Description of PR

How was this patch tested?

For code changes:

omalley commented May 5, 2022 • edited Loading

hchaverri commented May 6, 2022

hadoop-yetus commented May 6, 2022

slfan1989 commented May 7, 2022 • edited Loading

szilard-nemeth commented May 8, 2022

slfan1989 commented May 9, 2022 • edited Loading

szilard-nemeth commented May 10, 2022 • edited Loading

slfan1989 commented May 10, 2022 • edited Loading

omalley commented May 5, 2022 •

edited

Loading

slfan1989 commented May 7, 2022 •

edited

Loading

slfan1989 commented May 9, 2022 •

edited

Loading

szilard-nemeth commented May 10, 2022 •

edited

Loading

slfan1989 commented May 10, 2022 •

edited

Loading