You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/08/30 14:46:11 UTC
[GitHub] [accumulo] DomGarguilo opened a new pull request, #2902: Attempt to make KerberosRenewalIT more stable
DomGarguilo opened a new pull request, #2902:
URL: https://github.com/apache/accumulo/pull/2902
Recently, KerberosRenewalIT.testReadAndWriteThroughTicketLifetime had failed once on [a jenkins build](https://ci-builds.apache.org/job/Accumulo/job/main/org.apache.accumulo$accumulo-test/388/testReport/junit/org.apache.accumulo.test.functional/KerberosRenewalIT/testReadAndWriteThroughTicketLifetime/). I couldn't find record of this test failing in the past but I figure its an easy enough change to ensure it won't fail again.
Stacktrace:
<details>
```
org.apache.accumulo.core.client.TableExistsException: Table testReadAndWriteThroughTicketLifetime_table exists
at org.apache.accumulo.core.clientImpl.TableOperationsImpl.doFateOperation(TableOperationsImpl.java:391)
at org.apache.accumulo.core.clientImpl.TableOperationsImpl.doFateOperation(TableOperationsImpl.java:359)
at org.apache.accumulo.core.clientImpl.TableOperationsImpl.doTableFateOperation(TableOperationsImpl.java:1693)
at org.apache.accumulo.core.clientImpl.TableOperationsImpl.create(TableOperationsImpl.java:248)
at org.apache.accumulo.core.clientImpl.TableOperationsImpl.create(TableOperationsImpl.java:220)
at org.apache.accumulo.test.functional.KerberosRenewalIT.createReadWriteDrop(KerberosRenewalIT.java:190)
at org.apache.accumulo.test.functional.KerberosRenewalIT.testReadAndWriteThroughTicketLifetime(KerberosRenewalIT.java:171)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:566)
at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:727)
at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
at org.junit.jupiter.api.AssertTimeout.lambda$assertTimeoutPreemptively$4(AssertTimeout.java:138)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
at java.base/java.lang.Thread.run(Thread.java:834)
Caused by: ThriftTableOperationException(tableId:null, tableName:testReadAndWriteThroughTicketLifetime_table, op:CREATE, type:EXISTS, description:null)
at org.apache.accumulo.core.manager.thrift.FateService$waitForFateOperation_result$waitForFateOperation_resultStandardScheme.read(FateService.java:4896)
at org.apache.accumulo.core.manager.thrift.FateService$waitForFateOperation_result$waitForFateOperation_resultStandardScheme.read(FateService.java:4865)
at org.apache.accumulo.core.manager.thrift.FateService$waitForFateOperation_result.read(FateService.java:4791)
at org.apache.thrift.TServiceClient.receiveBase(TServiceClient.java:88)
at org.apache.accumulo.core.manager.thrift.FateService$Client.recv_waitForFateOperation(FateService.java:161)
at org.apache.accumulo.core.manager.thrift.FateService$Client.waitForFateOperation(FateService.java:146)
at org.apache.accumulo.core.clientImpl.TableOperationsImpl.waitForFateOperation(TableOperationsImpl.java:306)
at org.apache.accumulo.core.clientImpl.TableOperationsImpl.doFateOperation(TableOperationsImpl.java:375)
```
</details>
It looks like this test failed because the table already existed for some reason. This test repeatedly creates, uses and deletes a table with the same name. My guess is the table wasn't correctly deleted for some reason on one of those cycles.
The changes in this PR...
* create a method that will delete the table and recursively try to create it again if the table already exists.
* moves a delete table call outside of an unrelated try block.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] EdColeman commented on a diff in pull request #2902: Attempt to make KerberosRenewalIT more stable
Posted by GitBox <gi...@apache.org>.
EdColeman commented on code in PR #2902:
URL: https://github.com/apache/accumulo/pull/2902#discussion_r958610947
##########
test/src/main/java/org/apache/accumulo/test/functional/KerberosRenewalIT.java:
##########
@@ -200,7 +199,21 @@ private void createReadWriteDrop(AccumuloClient client) throws TableNotFoundExce
new Key("a", "b", "c").compareTo(entry.getKey(), PartialKey.ROW_COLFAM_COLQUAL),
"Did not find the expected key");
assertEquals("d", entry.getValue().toString());
- client.tableOperations().delete(table);
}
+ client.tableOperations().delete(table);
+ }
+
+ private String createTableAndReturnTableName(AccumuloClient client) throws AccumuloException,
+ AccumuloSecurityException, TableNotFoundException, TableExistsException {
+ final String tableName = getUniqueNames(1)[0] + "_table";
+ try {
+ client.tableOperations().create(tableName);
+ } catch (TableExistsException e) {
+ log.debug("Table {} already exists. Deleting and trying again.", tableName);
+ client.tableOperations().delete(tableName);
Review Comment:
Do tableOperations wait for the operation to complete - or do they create a FATE op and return? If the FATE op is launched, then this could end up with a race condition and end up throwing TableNotFound or another exception. If the FATE was in progress, the create fails because its there, the FATE completes and removes the table, then the delete fails becuase it does not exist. Another strategy may be to pull the create / delete into functions that retry and pause a little a few times.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo merged pull request #2902: Attempt to make KerberosRenewalIT more stable
Posted by GitBox <gi...@apache.org>.
DomGarguilo merged PR #2902:
URL: https://github.com/apache/accumulo/pull/2902
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo commented on a diff in pull request #2902: Attempt to make KerberosRenewalIT more stable
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #2902:
URL: https://github.com/apache/accumulo/pull/2902#discussion_r958916809
##########
test/src/main/java/org/apache/accumulo/test/functional/KerberosRenewalIT.java:
##########
@@ -200,7 +199,21 @@ private void createReadWriteDrop(AccumuloClient client) throws TableNotFoundExce
new Key("a", "b", "c").compareTo(entry.getKey(), PartialKey.ROW_COLFAM_COLQUAL),
"Did not find the expected key");
assertEquals("d", entry.getValue().toString());
- client.tableOperations().delete(table);
}
+ client.tableOperations().delete(table);
+ }
+
+ private String createTableAndReturnTableName(AccumuloClient client) throws AccumuloException,
+ AccumuloSecurityException, TableNotFoundException, TableExistsException {
+ final String tableName = getUniqueNames(1)[0] + "_table";
+ try {
+ client.tableOperations().create(tableName);
+ } catch (TableExistsException e) {
+ log.debug("Table {} already exists. Deleting and trying again.", tableName);
+ client.tableOperations().delete(tableName);
Review Comment:
That's a good point, and looking at the stack trace, probably how this test failed in the first place.
> Another strategy may be to pull the create / delete into functions that retry and pause a little a few times.
This seems like a good idea. Also seems like a perfect place for the [`Wait.java`](https://github.com/cshannon/accumulo/blob/9d7a69d5ec4fb35c5f882a64596e7843ccbe886e/test/src/main/java/org/apache/accumulo/test/util/Wait.java#L28) test util that is part of #2799, added by @cshannon. I'm wondering if that could be broken into its own PR so it can be used elsewhere while #2799 is still under review.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #2902: Attempt to make KerberosRenewalIT more stable
Posted by GitBox <gi...@apache.org>.
dlmarion commented on code in PR #2902:
URL: https://github.com/apache/accumulo/pull/2902#discussion_r958755439
##########
test/src/main/java/org/apache/accumulo/test/functional/KerberosRenewalIT.java:
##########
@@ -200,7 +199,21 @@ private void createReadWriteDrop(AccumuloClient client) throws TableNotFoundExce
new Key("a", "b", "c").compareTo(entry.getKey(), PartialKey.ROW_COLFAM_COLQUAL),
"Did not find the expected key");
assertEquals("d", entry.getValue().toString());
- client.tableOperations().delete(table);
}
+ client.tableOperations().delete(table);
Review Comment:
I don't know that it would be an improvement as you are getting a unique table name at the start of the test. If this test fails, it's unlikely that the table existing would cause an issue with another test or this test being repeated. If this were not test code, I would say putting it in a finally block would be needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo commented on a diff in pull request #2902: Attempt to make KerberosRenewalIT more stable
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #2902:
URL: https://github.com/apache/accumulo/pull/2902#discussion_r958734038
##########
test/src/main/java/org/apache/accumulo/test/functional/KerberosRenewalIT.java:
##########
@@ -200,7 +199,21 @@ private void createReadWriteDrop(AccumuloClient client) throws TableNotFoundExce
new Key("a", "b", "c").compareTo(entry.getKey(), PartialKey.ROW_COLFAM_COLQUAL),
"Did not find the expected key");
assertEquals("d", entry.getValue().toString());
- client.tableOperations().delete(table);
}
+ client.tableOperations().delete(table);
Review Comment:
Do you think that would be an improvement? It can be easily added if so.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #2902: Attempt to make KerberosRenewalIT more stable
Posted by GitBox <gi...@apache.org>.
dlmarion commented on code in PR #2902:
URL: https://github.com/apache/accumulo/pull/2902#discussion_r958709219
##########
test/src/main/java/org/apache/accumulo/test/functional/KerberosRenewalIT.java:
##########
@@ -200,7 +199,21 @@ private void createReadWriteDrop(AccumuloClient client) throws TableNotFoundExce
new Key("a", "b", "c").compareTo(entry.getKey(), PartialKey.ROW_COLFAM_COLQUAL),
"Did not find the expected key");
assertEquals("d", entry.getValue().toString());
- client.tableOperations().delete(table);
}
+ client.tableOperations().delete(table);
Review Comment:
this could be in a finally block, but likely not necessary since this is a test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo commented on pull request #2902: Attempt to make KerberosRenewalIT more stable
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on PR #2902:
URL: https://github.com/apache/accumulo/pull/2902#issuecomment-1231975021
@dlmarion, your suggestions should be addressed now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion commented on a diff in pull request #2902: Attempt to make KerberosRenewalIT more stable
Posted by GitBox <gi...@apache.org>.
dlmarion commented on code in PR #2902:
URL: https://github.com/apache/accumulo/pull/2902#discussion_r958721634
##########
test/src/main/java/org/apache/accumulo/test/functional/KerberosRenewalIT.java:
##########
@@ -200,7 +199,21 @@ private void createReadWriteDrop(AccumuloClient client) throws TableNotFoundExce
new Key("a", "b", "c").compareTo(entry.getKey(), PartialKey.ROW_COLFAM_COLQUAL),
"Did not find the expected key");
assertEquals("d", entry.getValue().toString());
- client.tableOperations().delete(table);
}
+ client.tableOperations().delete(table);
+ }
+
+ private String createTableAndReturnTableName(AccumuloClient client) throws AccumuloException,
+ AccumuloSecurityException, TableNotFoundException, TableExistsException {
+ final String tableName = getUniqueNames(1)[0] + "_table";
+ try {
+ client.tableOperations().create(tableName);
+ } catch (TableExistsException e) {
+ log.debug("Table {} already exists. Deleting and trying again.", tableName);
+ client.tableOperations().delete(tableName);
+ createTableAndReturnTableName(client);
Review Comment:
```suggestion
tableName = createTableAndReturnTableName(client);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] DomGarguilo commented on a diff in pull request #2902: Attempt to make KerberosRenewalIT more stable
Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #2902:
URL: https://github.com/apache/accumulo/pull/2902#discussion_r965123038
##########
test/src/main/java/org/apache/accumulo/test/functional/KerberosRenewalIT.java:
##########
@@ -200,7 +199,21 @@ private void createReadWriteDrop(AccumuloClient client) throws TableNotFoundExce
new Key("a", "b", "c").compareTo(entry.getKey(), PartialKey.ROW_COLFAM_COLQUAL),
"Did not find the expected key");
assertEquals("d", entry.getValue().toString());
- client.tableOperations().delete(table);
}
+ client.tableOperations().delete(table);
+ }
+
+ private String createTableAndReturnTableName(AccumuloClient client) throws AccumuloException,
+ AccumuloSecurityException, TableNotFoundException, TableExistsException {
+ final String tableName = getUniqueNames(1)[0] + "_table";
+ try {
+ client.tableOperations().create(tableName);
+ } catch (TableExistsException e) {
+ log.debug("Table {} already exists. Deleting and trying again.", tableName);
+ client.tableOperations().delete(tableName);
Review Comment:
@EdColeman, this should be resolved as of [bdca3bd](https://github.com/apache/accumulo/pull/2902/commits/bdca3bde1fc9fef0034d9ebc2d1b6263f6d096d0).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org