[
https://issues.apache.org/jira/browse/HADOOP-12611?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15526620#comment-15526620
]
Eric Badger edited comment on HADOOP-12611 at 9/27/16 4:20 PM:
---------------------------------------------------------------
[~rkanter], thanks for the detailed response! What you said makes sense with
regards to the race between the servers calling rollSecret() to push their
secrets to ZK. Let me make sure that I understand the approach that you
proposed above.
1. Seed deterministically as we are currently doing and let rollSecret() happen
twice.
- You said above to check secrets based on size, but the secrets list is only
ever 2 elements. So I could check for it to change, but I don't know how I
would check for each iteration based on the size of the list.
2. Keep track of the secrets list for both A and B at each iteration.
3. Check to make sure that A and B are correct at each iteration
- A: [A1, null], [A2, A1], [A3 or B3, A2]
- B: [A2, A1], [A3 or B3, A2]
I do see a potential problem with this setup though. Right after we call
secretProviderB.init(), we check to make sure that it's secrets are equal to
[A2, A1]. But if there is a slow code path for whatever reason in the main
code, then rollSecret() could be called to update the secrets via either
secretProviderA or secretProviderB. This would make the secrets [A3 or B3, A2]
(or something else if rollSecret() was called multiple times) instead of [A2,
A1]. I'm not sure how to remove this race condition without changing the source
code.
A little hokey, but would it be acceptable to explicitly call rollSecret()
instead of using verify(), but calling them in a random order? This way we
guarantee the number of times that rollSecret() is called, we guarantee the
contents of secrets for both secretProviders, and we still provide the
randomness of each secretProvider being able to talk to ZK first.
was (Author: ebadger):
[~rkanter], thanks for the detailed response! What you said makes sense with
regards to the race between the servers calling rollSecret() to push their
secrets to ZK. Let me make sure that I understand the approach that you
proposed above.
1. Seed deterministically as we are currently doing and let rollSecret() happen
twice.
- You said above to check secrets based on size, but the secrets list is only
ever 2 elements. So I could check for it to change, but I don't know how I
would check for each iteration based on the size of the list.
2. Keep track of the secrets list for both A and B at each iteration.
3. Check to make sure that A and B are correct at each iteration
- A: [A1, null], [A2, A1], [A3 or B3, A2]
- B: [A2, A1], [A3 or B3, A2]
I do see a potential problem with this setup though. Right after we call
secretProviderB.init(), we check to make sure that it's secrets are equal to
[A2, A1]. But if there is a slow code path for whatever reason in the main
code, then rollSecret() could be called to update the secrets via either
secretProviderA or secretProviderB. This would make the secrets [A3 or B3, A2]
(or something else if rollSecret() was called multiple times) instead of [A2,
A1]. I'm not sure how to remove this race condition without changing the source
code.
A little hokey, but would it be acceptable to explicitly call rollSecret()
instead of using verify(), but calling them in a random order? This way we
guarantee the number of times that rollSecret() is called, we guarantee the
contents of secrets for both secretProviders, and we still provide the
randomness of each secretProvider being able to talk to ZK first.
> TestZKSignerSecretProvider#testMultipleInit occasionally fail
> -------------------------------------------------------------
>
> Key: HADOOP-12611
> URL: https://issues.apache.org/jira/browse/HADOOP-12611
> Project: Hadoop Common
> Issue Type: Bug
> Reporter: Wei-Chiu Chuang
> Assignee: Wei-Chiu Chuang
> Attachments: HADOOP-12611.001.patch, HADOOP-12611.002.patch
>
>
> https://builds.apache.org/job/Hadoop-Common-trunk/2053/testReport/junit/org.apache.hadoop.security.authentication.util/TestZKSignerSecretProvider/testMultipleInit/
> Error Message
> expected null, but was:<[B@142bad79>
> Stacktrace
> java.lang.AssertionError: expected null, but was:<[B@142bad79>
> at org.junit.Assert.fail(Assert.java:88)
> at org.junit.Assert.failNotNull(Assert.java:664)
> at org.junit.Assert.assertNull(Assert.java:646)
> at org.junit.Assert.assertNull(Assert.java:656)
> at
> org.apache.hadoop.security.authentication.util.TestZKSignerSecretProvider.testMultipleInit(TestZKSignerSecretProvider.java:149)
> I think the failure was introduced after HADOOP-12181
> This is likely where the root cause is:
> 2015-11-29 00:24:33,325 ERROR ZKSignerSecretProvider - An unexpected
> exception occurred while pulling data fromZooKeeper
> java.lang.IllegalStateException: instance must be started before calling this
> method
> at
> com.google.common.base.Preconditions.checkState(Preconditions.java:145)
> at
> org.apache.curator.framework.imps.CuratorFrameworkImpl.getData(CuratorFrameworkImpl.java:363)
> at
> org.apache.hadoop.security.authentication.util.ZKSignerSecretProvider.pullFromZK(ZKSignerSecretProvider.java:341)
> at
> org.apache.hadoop.security.authentication.util.ZKSignerSecretProvider.rollSecret(ZKSignerSecretProvider.java:264)
> at
> org.apache.hadoop.security.authentication.util.ZKSignerSecretProvider$$EnhancerByMockitoWithCGLIB$$575f06d8.CGLIB$rollSecret$2(<generated>)
> at
> org.apache.hadoop.security.authentication.util.ZKSignerSecretProvider$$EnhancerByMockitoWithCGLIB$$575f06d8$$FastClassByMockitoWithCGLIB$$6f94a716.invoke(<generated>)
> at org.mockito.cglib.proxy.MethodProxy.invokeSuper(MethodProxy.java:216)
> at
> org.mockito.internal.creation.AbstractMockitoMethodProxy.invokeSuper(AbstractMockitoMethodProxy.java:10)
> at
> org.mockito.internal.invocation.realmethod.CGLIBProxyRealMethod.invoke(CGLIBProxyRealMethod.java:22)
> at
> org.mockito.internal.invocation.realmethod.FilteredCGLIBProxyRealMethod.invoke(FilteredCGLIBProxyRealMethod.java:27)
> at
> org.mockito.internal.invocation.Invocation.callRealMethod(Invocation.java:211)
> at
> org.mockito.internal.stubbing.answers.CallsRealMethods.answer(CallsRealMethods.java:36)
> at org.mockito.internal.MockHandler.handle(MockHandler.java:99)
> at
> org.mockito.internal.creation.MethodInterceptorFilter.intercept(MethodInterceptorFilter.java:47)
> at
> org.apache.hadoop.security.authentication.util.ZKSignerSecretProvider$$EnhancerByMockitoWithCGLIB$$575f06d8.rollSecret(<generated>)
> at
> org.apache.hadoop.security.authentication.util.RolloverSignerSecretProvider$1.run(RolloverSignerSecretProvider.java:97)
> at
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
> at java.util.concurrent.FutureTask.runAndReset(FutureTask.java:304)
> at
> java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$301(ScheduledThreadPoolExecutor.java:178)
> at
> java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
> at java.lang.Thread.run(Thread.java:745)
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
