You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "cshannon (via GitHub)" <gi...@apache.org> on 2024/01/26 17:28:04 UTC
[PR] Make sure AccumuloStore persists ageoff txinfo [accumulo]
cshannon opened a new pull request, #4199:
URL: https://github.com/apache/accumulo/pull/4199
The changes in #4169 moved age off tracking out of memory and into the FATE store. The AccumuloStore has a bug where it doesn't handle the new TxInfo enum type of TX_AGEOFF so the info is never persisted to the store. This fixes the store so it will correctly read/write the value and also adds a default enum case to throw an exception in the future in case a new value is passed that is unknown so it will be caught. A test was also added to iterate over all types to verify they all work for the AccumuloStore so if a new info type is ever added it will validate it in the future.
--
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
Re: [PR] Make sure AccumuloStore persists ageoff txinfo [accumulo]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on PR #4199:
URL: https://github.com/apache/accumulo/pull/4199#issuecomment-1912576170
I'm going to merge this now and then create a follow on PR so that the same tests that run for the Accumulo store in `AccumuloStoreReadWriteIT` will be run against the Zoostore too
--
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
Re: [PR] Make sure AccumuloStore persists ageoff txinfo [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #4199:
URL: https://github.com/apache/accumulo/pull/4199#discussion_r1468035242
##########
test/src/main/java/org/apache/accumulo/test/fate/accumulo/AccumuloStoreReadWriteIT.java:
##########
@@ -127,6 +127,31 @@ public void testReadWrite() throws Exception {
}
}
+ @Test
+ public void testReadWriteTxInfo() throws Exception {
+ final String table = getUniqueNames(1)[0];
+ try (ClientContext client =
+ (ClientContext) Accumulo.newClient().from(getClientProps()).build()) {
+ client.tableOperations().create(table);
+
+ AccumuloStore<TestEnv> store = new AccumuloStore<>(client, table);
+
+ long tid = store.create();
+ FateTxStore<TestEnv> txStore = store.reserve(tid);
+
+ try {
+ // Go through all enum values to verify each TxInfo type will be properly
+ // written and read from the store
+ for (TxInfo txInfo : TxInfo.values()) {
+ txStore.setTransactionInfo(txInfo, "value: " + txInfo.name());
+ assertEquals("value: " + txInfo.name(), txStore.getTransactionInfo(txInfo));
+ }
+ } finally {
+ txStore.delete();
Review Comment:
I was trying to think if there is anything that should be checked after delete, but did not think of anything.
##########
test/src/main/java/org/apache/accumulo/test/fate/accumulo/AccumuloStoreReadWriteIT.java:
##########
@@ -127,6 +127,31 @@ public void testReadWrite() throws Exception {
}
}
+ @Test
+ public void testReadWriteTxInfo() throws Exception {
+ final String table = getUniqueNames(1)[0];
+ try (ClientContext client =
+ (ClientContext) Accumulo.newClient().from(getClientProps()).build()) {
+ client.tableOperations().create(table);
+
+ AccumuloStore<TestEnv> store = new AccumuloStore<>(client, table);
+
+ long tid = store.create();
+ FateTxStore<TestEnv> txStore = store.reserve(tid);
+
+ try {
+ // Go through all enum values to verify each TxInfo type will be properly
+ // written and read from the store
+ for (TxInfo txInfo : TxInfo.values()) {
+ txStore.setTransactionInfo(txInfo, "value: " + txInfo.name());
Review Comment:
Could add a check before setting to check that case.
```suggestion
assertNull(txStore.getTransactionInfo(txInfo));
txStore.setTransactionInfo(txInfo, "value: " + txInfo.name());
```
--
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
Re: [PR] Make sure AccumuloStore persists ageoff txinfo [accumulo]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon merged PR #4199:
URL: https://github.com/apache/accumulo/pull/4199
--
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