Skip to content

Conversation

@hanishakoneru
Copy link
Contributor

What changes were proposed in this pull request?

If a client attempts a request on an OM which has not caught up with leader OM and hence does have the delegation token, the request could fail with AccessControlException without trying it on other OMs.
On AccessControlException, all OMs must be tried once before the request is failed.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-4075

How was this patch tested?

No new unit tests

Copy link
Contributor

@bharatviswa504 bharatviswa504 left a comment

Choose a reason for hiding this comment

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

Overall LGTM.
Can we add a test for this to verify this behavior?

But looks like we do not have any secure Ozone HA cluster, need to see other ways for tests on this behavior.

Copy link
Contributor

@bharatviswa504 bharatviswa504 left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Few minor comments.


OzoneManagerProtocolPB proxy = (OzoneManagerProtocolPB) RetryProxy
.create(OzoneManagerProtocolPB.class, failoverProxyProvider,
failoverProxyProvider.getRetryPolicy(9));
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: Can we use the default, if there is no special reason to use value 9

private Exception testException;

@Test
public void test1() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: Can we name test with the test it is trying to perform like testAccessControlException

@bharatviswa504
Copy link
Contributor

Thank You @hanishakoneru for the update and refactor code, and making it easy for Unit test FailOverProxy Provider.

Copy link
Contributor

@bharatviswa504 bharatviswa504 left a comment

Choose a reason for hiding this comment

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

+1 LGTM

@bharatviswa504 bharatviswa504 merged commit 045aa71 into apache:master Sep 15, 2020
@bharatviswa504
Copy link
Contributor

Thank You @hanishakoneru for the contribution.

errose28 added a commit to errose28/ozone that referenced this pull request Sep 18, 2020
* master: (47 commits)
  HDDS-4104. Provide a way to get the default value and key of java-based-configuration easily (apache#1369)
  HDDS-4250. Fix wrong logger name (apache#1429)
  HDDS-4244. Container deleted wrong replica cause mis-replicated. (apache#1423)
  HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key. (apache#1296)
  HDDS-4210. ResolveBucket during checkAcls fails. (apache#1398)
  HDDS-4075. Retry request on different OM on AccessControlException (apache#1303)
  HDDS-4166. Documentation index page redirects to the wrong address (apache#1372)
  HDDS-4039. Reduce the number of fields in hdds.proto to improve performance (apache#1289)
  HDDS-4155. Directory and filename can end up with same name in a path. (apache#1361)
  HDDS-3927. Rename Ozone OM,DN,SCM runtime options to conform to naming conventions (apache#1401)
  HDDS-4119. Improve performance of the BufferPool management of Ozone client (apache#1336)
  HDDS-4217.Remove test TestOzoneContainerRatis (apache#1408)
  HDDS-4218.Remove test TestRatisManager (apache#1409)
  HDDS-4129. change MAX_QUOTA_IN_BYTES to Long.MAX_VALUE. (apache#1337)
  HDDS-4228: add field 'num' to ALLOCATE_BLOCK of scm audit log. (apache#1413)
  HDDS-4196. Add an endpoint in Recon to query Prometheus (apache#1390)
  HDDS-4211. [OFS] Better owner and group display for listing Ozone volumes and buckets (apache#1397)
  HDDS-4150. recon.api.TestEndpoints test is flaky (apache#1396)
  HDDS-4170 - Fix typo in method description. (apache#1406)
  HDDS-4064. Show container verbose info with verbose option (apache#1290)
  ...
errose28 added a commit to errose28/ozone that referenced this pull request Sep 18, 2020
…ponse

* HDDS-4122-quota-attempt2: (51 commits)
  Remove redundant check status calls in children of AbstractOMOpenKeyDeleteRequest
  Remove unused inports and fix super constructor calls
  Move common volume byte usage update code to AbstractOMKeyDeleteResponse
  Add volume quota update to open key delete response, and group duplicate code
  HDDS-4104. Provide a way to get the default value and key of java-based-configuration easily (apache#1369)
  HDDS-4250. Fix wrong logger name (apache#1429)
  HDDS-4244. Container deleted wrong replica cause mis-replicated. (apache#1423)
  HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key. (apache#1296)
  HDDS-4210. ResolveBucket during checkAcls fails. (apache#1398)
  HDDS-4075. Retry request on different OM on AccessControlException (apache#1303)
  HDDS-4166. Documentation index page redirects to the wrong address (apache#1372)
  HDDS-4039. Reduce the number of fields in hdds.proto to improve performance (apache#1289)
  HDDS-4155. Directory and filename can end up with same name in a path. (apache#1361)
  HDDS-3927. Rename Ozone OM,DN,SCM runtime options to conform to naming conventions (apache#1401)
  HDDS-4119. Improve performance of the BufferPool management of Ozone client (apache#1336)
  HDDS-4217.Remove test TestOzoneContainerRatis (apache#1408)
  HDDS-4218.Remove test TestRatisManager (apache#1409)
  HDDS-4129. change MAX_QUOTA_IN_BYTES to Long.MAX_VALUE. (apache#1337)
  HDDS-4228: add field 'num' to ALLOCATE_BLOCK of scm audit log. (apache#1413)
  HDDS-4196. Add an endpoint in Recon to query Prometheus (apache#1390)
  ...
@hanishakoneru hanishakoneru deleted the HDDS-4075 branch December 1, 2020 21:25
Sign up for free to 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.

2 participants