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/06/04 00:55:29 UTC
[GitHub] [accumulo] EdColeman opened a new pull request, #2755: modify filter conditions for status
EdColeman opened a new pull request, #2755:
URL: https://github.com/apache/accumulo/pull/2755
This is more for discussion of #2754, the the fate -print issue. It seems that the code was filtering the transaction.
Comparing the logic in main and in https://github.com/apache/accumulo/pull/2215, the logic seems the same, so not sure what is goingon.
Also, the original logic seemed correct, but rather than the negative / skip logic that was used, this code flips that. Not sure this is a final solution - but maybe it help point to the issue.
--
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 #2755: modify filter conditions for status
Posted by GitBox <gi...@apache.org>.
EdColeman commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r891133185
##########
core/src/main/java/org/apache/accumulo/fate/AdminUtil.java:
##########
@@ -390,11 +390,10 @@ private FateStatus getTransactionStatus(ReadOnlyTStore<T> zs, Set<Long> filterTx
zs.unreserve(tid, 0);
- if ((filterTxid != null && !filterTxid.contains(tid))
- || (filterStatus != null && !filterStatus.contains(status)))
- continue;
-
- statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ if ((filterTxid == null) || filterTxid.contains(tid) || (filterStatus == null)
+ || filterStatus.contains(status)) {
+ statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ }
Review Comment:
That's reasonable, but they way it was working was that if you provided a txid and a status, you would get the txid if it matched and any fates that matched that state regardless of the txid.
With this version, you could look for a specific txid as long as it was in a stage say txid = 0 and IN_PROGRESS, that seemed better than txid = 0, and all other fates that are IN_PROGRESS
--
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] milleruntime commented on a diff in pull request #2755: Fix Fate print command and improve ShellServerIT test
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r903612637
##########
core/src/main/java/org/apache/accumulo/fate/AdminUtil.java:
##########
@@ -390,17 +388,23 @@ private FateStatus getTransactionStatus(ReadOnlyTStore<T> zs, Set<Long> filterTx
zs.unreserve(tid, 0);
- if ((filterTxid != null && !filterTxid.contains(tid))
- || (filterStatus != null && !filterStatus.contains(status)))
- continue;
-
- statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ if (includeByStatus(status, filterStatus) && includeByTxid(tid, filterTxid)) {
Review Comment:
> I thought the discussion centered around the includeByTxid && includeByStatus and if that should be an or. The current behavior in this code is that if you specify txid and a status. both must match. (and both take a list)
Yes.
I am going to take a look at the new Test today and see if I can come up with the untested case as well.
--
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 #2755: Fix Fate print command and improve ShellServerIT test
Posted by GitBox <gi...@apache.org>.
EdColeman commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r902997562
##########
core/src/main/java/org/apache/accumulo/fate/AdminUtil.java:
##########
@@ -390,17 +388,23 @@ private FateStatus getTransactionStatus(ReadOnlyTStore<T> zs, Set<Long> filterTx
zs.unreserve(tid, 0);
- if ((filterTxid != null && !filterTxid.contains(tid))
- || (filterStatus != null && !filterStatus.contains(status)))
- continue;
-
- statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ if (includeByStatus(status, filterStatus) && includeByTxid(tid, filterTxid)) {
Review Comment:
This line in the added test expects that no arguments prints all
```
ts.exec("fate -print", true, "txid:", true);
```
This line in the added test expects that no txid but a status return results (on match):
```
ts.exec("fate -print -t IN_PROGRESS", true, "txid:", true);
```
The case of just a txid (success) is untested, but verified manually because getting the txid was not straight forward.
I thought the discussion centered around the `includeByTxid && includeByStatus` and if that should be an or. The current behavior in this code is that if you specify txid and a status. both must match. (and both take a list)
--
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] milleruntime commented on a diff in pull request #2755: modify filter conditions for status
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r891098998
##########
core/src/main/java/org/apache/accumulo/fate/AdminUtil.java:
##########
@@ -390,11 +390,10 @@ private FateStatus getTransactionStatus(ReadOnlyTStore<T> zs, Set<Long> filterTx
zs.unreserve(tid, 0);
- if ((filterTxid != null && !filterTxid.contains(tid))
- || (filterStatus != null && !filterStatus.contains(status)))
- continue;
-
- statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ if ((filterTxid == null) || filterTxid.contains(tid) || (filterStatus == null)
+ || filterStatus.contains(status)) {
+ statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ }
Review Comment:
It doesn't make sense to filter by both the txid AND the status. The txid should be unique. We should probably throw an error if the user provides both. Or if the user provides the txid, just filter by that.
--
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 #2755: modify filter conditions for status
Posted by GitBox <gi...@apache.org>.
EdColeman commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r890185929
##########
core/src/main/java/org/apache/accumulo/fate/AdminUtil.java:
##########
@@ -390,11 +390,10 @@ private FateStatus getTransactionStatus(ReadOnlyTStore<T> zs, Set<Long> filterTx
zs.unreserve(tid, 0);
- if ((filterTxid != null && !filterTxid.contains(tid))
- || (filterStatus != null && !filterStatus.contains(status)))
- continue;
-
- statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ if ((filterTxid == null) || filterTxid.contains(tid) || (filterStatus == null)
+ || filterStatus.contains(status)) {
+ statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ }
Review Comment:
For me, yes. Can you verify by adding the accumulo.properties where you were testing / seeing the issue?
What I don't understand is why the filter seemed to change - needing the properties for a test using fate print is understandable, but the same filter code seems to be behavior seems to differ between the two branch and I don't see why. That's why I set this to draft until the difference can be explained.
--
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] milleruntime commented on a diff in pull request #2755: modify filter conditions for status
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r890160260
##########
core/src/main/java/org/apache/accumulo/fate/AdminUtil.java:
##########
@@ -390,11 +390,10 @@ private FateStatus getTransactionStatus(ReadOnlyTStore<T> zs, Set<Long> filterTx
zs.unreserve(tid, 0);
- if ((filterTxid != null && !filterTxid.contains(tid))
- || (filterStatus != null && !filterStatus.contains(status)))
- continue;
-
- statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ if ((filterTxid == null) || filterTxid.contains(tid) || (filterStatus == null)
+ || filterStatus.contains(status)) {
+ statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ }
Review Comment:
Does the test fail without this change?
--
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] milleruntime commented on a diff in pull request #2755: modify filter conditions for status
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r890357747
##########
core/src/main/java/org/apache/accumulo/fate/AdminUtil.java:
##########
@@ -390,11 +390,10 @@ private FateStatus getTransactionStatus(ReadOnlyTStore<T> zs, Set<Long> filterTx
zs.unreserve(tid, 0);
- if ((filterTxid != null && !filterTxid.contains(tid))
- || (filterStatus != null && !filterStatus.contains(status)))
- continue;
-
- statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ if ((filterTxid == null) || filterTxid.contains(tid) || (filterStatus == null)
+ || filterStatus.contains(status)) {
+ statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ }
Review Comment:
Yeah, I am not sure what is going on either. I need to take a closer look at this logic.
--
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 merged pull request #2755: Fix Fate print command and improve ShellServerIT test
Posted by GitBox <gi...@apache.org>.
EdColeman merged PR #2755:
URL: https://github.com/apache/accumulo/pull/2755
--
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 #2755: modify filter conditions for status
Posted by GitBox <gi...@apache.org>.
EdColeman commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r891268937
##########
core/src/main/java/org/apache/accumulo/fate/AdminUtil.java:
##########
@@ -390,11 +390,10 @@ private FateStatus getTransactionStatus(ReadOnlyTStore<T> zs, Set<Long> filterTx
zs.unreserve(tid, 0);
- if ((filterTxid != null && !filterTxid.contains(tid))
- || (filterStatus != null && !filterStatus.contains(status)))
- continue;
-
- statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ if ((filterTxid == null) || filterTxid.contains(tid) || (filterStatus == null)
+ || filterStatus.contains(status)) {
+ statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ }
Review Comment:
0 was a standing for any txid, could be 1234, whatever. The point was that specifying a set of txids and a state, then I would expect that it would be an AND - only the txids that match and only in the state provided.
--
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] milleruntime commented on a diff in pull request #2755: modify filter conditions for status
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r891237960
##########
core/src/main/java/org/apache/accumulo/fate/AdminUtil.java:
##########
@@ -390,11 +390,10 @@ private FateStatus getTransactionStatus(ReadOnlyTStore<T> zs, Set<Long> filterTx
zs.unreserve(tid, 0);
- if ((filterTxid != null && !filterTxid.contains(tid))
- || (filterStatus != null && !filterStatus.contains(status)))
- continue;
-
- statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ if ((filterTxid == null) || filterTxid.contains(tid) || (filterStatus == null)
+ || filterStatus.contains(status)) {
+ statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ }
Review Comment:
> That's reasonable, but they way it was working was that if you provided a txid and a status, you would get the txid if it matched and any fates that matched that state regardless of the txid.
Working in a prior released version? like 1.10? If that's what the behavior was the we should preserve the current behavior.
> With this version, you could look for a specific txid as long as it was in a stage say txid = 0 and IN_PROGRESS, that seemed better than txid = 0, and all other fates that are IN_PROGRESS
I don't follow you. I don't think the user will be able to search on txid = 0. One of the first things FATE does is create the txid.
--
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] milleruntime commented on a diff in pull request #2755: modify filter conditions for status
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r890447312
##########
core/src/main/java/org/apache/accumulo/fate/AdminUtil.java:
##########
@@ -390,11 +390,10 @@ private FateStatus getTransactionStatus(ReadOnlyTStore<T> zs, Set<Long> filterTx
zs.unreserve(tid, 0);
- if ((filterTxid != null && !filterTxid.contains(tid))
- || (filterStatus != null && !filterStatus.contains(status)))
- continue;
-
- statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ if ((filterTxid == null) || filterTxid.contains(tid) || (filterStatus == null)
+ || filterStatus.contains(status)) {
+ statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ }
Review Comment:
Just the test on main fails without this change:
<pre>
[ERROR] Failures:
[ERROR] ShellServerIT.testFateCommandWithSlowCompaction:2119 txid: present in root@miniInstance ShellServerIT_testFateCommandWithSlowCompaction0> fate -print
0 transactions
was not true ==> expected: <true> but was: <false>
</pre>
--
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] milleruntime commented on pull request #2755: modify filter conditions for status
Posted by GitBox <gi...@apache.org>.
milleruntime commented on PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#issuecomment-1162010585
I think this could be marked ready for review. There are some white space changes that could be dropped and the logic is confusing but if the test is passing then that is the important part to fix #2754. This PR is definitely closer to being merged than #2780, which is just an updated version of [2215](https://github.com/apache/accumulo/pull/2215).
--
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] milleruntime commented on a diff in pull request #2755: Fix Fate print command and improve ShellServerIT test
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r902969741
##########
core/src/main/java/org/apache/accumulo/fate/AdminUtil.java:
##########
@@ -390,17 +388,23 @@ private FateStatus getTransactionStatus(ReadOnlyTStore<T> zs, Set<Long> filterTx
zs.unreserve(tid, 0);
- if ((filterTxid != null && !filterTxid.contains(tid))
- || (filterStatus != null && !filterStatus.contains(status)))
- continue;
-
- statuses.add(new TransactionStatus(tid, status, debug, hlocks, wlocks, top, timeCreated));
+ if (includeByStatus(status, filterStatus) && includeByTxid(tid, filterTxid)) {
Review Comment:
So if the user doesn't specify a TXID, will they get any results back for just a status? I think we may have talked about this but the AND here makes it seem like you need both to get anything back.
--
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] milleruntime commented on a diff in pull request #2755: modify filter conditions for status
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r890133821
##########
test/src/main/java/org/apache/accumulo/test/shell/ShellServerIT.java:
##########
@@ -2082,4 +2082,42 @@ public void testSummarySelection() throws Exception {
// check that there are two files, with none having extra summary info
assertMatches(output, "(?sm).*^.*total[:]2[,]\\s+missing[:]0[,]\\s+extra[:]0.*$.*");
}
+
+ @Test
+ public void testFateCommandWithSlowCompaction() throws Exception {
+ final String table = getUniqueNames(1)[0];
+
+ System.setProperty("accumulo.properties",
+ "file://" + getCluster().getConfig().getAccumuloPropsFile().getCanonicalPath());
Review Comment:
Nice!
--
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] milleruntime commented on a diff in pull request #2755: Fix Fate print command and improve ShellServerIT test
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r903623432
##########
test/src/main/java/org/apache/accumulo/test/shell/ShellServerIT.java:
##########
@@ -2082,4 +2082,54 @@ public void testSummarySelection() throws Exception {
// check that there are two files, with none having extra summary info
assertMatches(output, "(?sm).*^.*total[:]2[,]\\s+missing[:]0[,]\\s+extra[:]0.*$.*");
}
+
+ @Test
+ public void testFateCommandWithSlowCompaction() throws Exception {
+ final String table = getUniqueNames(1)[0];
+
+ String orgProps = System.getProperty("accumulo.properties");
+
+ System.setProperty("accumulo.properties",
+ "file://" + getCluster().getConfig().getAccumuloPropsFile().getCanonicalPath());
+ // compact
+ ts.exec("createtable " + table);
+
+ // setup SlowIterator to sleep for 10 seconds
+ ts.exec("config -t " + table
+ + " -s table.iterator.majc.slow=1,org.apache.accumulo.test.functional.SlowIterator");
+ ts.exec("config -t " + table + " -s table.iterator.majc.slow.opt.sleepTime=10000");
+
+ String tableId = getTableId(table);
+
+ // make two files
+ ts.exec("insert a1 b c v_a1");
+ ts.exec("insert a2 b c v_a2");
+ ts.exec("flush -w");
+ ts.exec("insert x1 b c v_x1");
+ ts.exec("insert x2 b c v_x2");
+ ts.exec("flush -w");
+ int oldCount = countFiles(tableId);
+
+ // merge two files into one
+ ts.exec("compact -t " + table);
+
+ Thread.sleep(3_000);
+ // compaction should still be running
+
+ log.info("Calling fate print for table = {}", table);
+ ts.exec("fate -print", true, "txid:", true);
+
+ // test filters
+ ts.exec("fate -print -t IN_PROGRESS", true, "txid:", true);
+ ts.exec("fate -print -t NEW", true, "0 transactions", true);
Review Comment:
This should work...
```suggestion
ts.exec("compact -t " + table);
// 2 compactions should be running so parse the output to get one of the transaction ids
log.info("Calling fate print for table = {}", table);
String result = ts.exec("fate -print", true, "txid:", true);
String[] resultParts = result.split("txid: ");
String[] parts = resultParts[1].split(" ");
String txid = parts[0];
// test filters
ts.exec("fate -print " + txid + " -t IN_PROGRESS", true, "1 transactions", true);
ts.exec("fate -print -t IN_PROGRESS", true, "2 transactions", true);
ts.exec("fate -print -t NEW", true, "0 transactions", true);
```
--
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 #2755: Fix Fate print command and improve ShellServerIT test
Posted by GitBox <gi...@apache.org>.
EdColeman commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r903738175
##########
test/src/main/java/org/apache/accumulo/test/shell/ShellServerIT.java:
##########
@@ -2082,4 +2082,54 @@ public void testSummarySelection() throws Exception {
// check that there are two files, with none having extra summary info
assertMatches(output, "(?sm).*^.*total[:]2[,]\\s+missing[:]0[,]\\s+extra[:]0.*$.*");
}
+
+ @Test
+ public void testFateCommandWithSlowCompaction() throws Exception {
+ final String table = getUniqueNames(1)[0];
+
+ String orgProps = System.getProperty("accumulo.properties");
+
+ System.setProperty("accumulo.properties",
+ "file://" + getCluster().getConfig().getAccumuloPropsFile().getCanonicalPath());
+ // compact
+ ts.exec("createtable " + table);
+
+ // setup SlowIterator to sleep for 10 seconds
+ ts.exec("config -t " + table
+ + " -s table.iterator.majc.slow=1,org.apache.accumulo.test.functional.SlowIterator");
+ ts.exec("config -t " + table + " -s table.iterator.majc.slow.opt.sleepTime=10000");
+
+ String tableId = getTableId(table);
+
+ // make two files
+ ts.exec("insert a1 b c v_a1");
+ ts.exec("insert a2 b c v_a2");
+ ts.exec("flush -w");
+ ts.exec("insert x1 b c v_x1");
+ ts.exec("insert x2 b c v_x2");
+ ts.exec("flush -w");
+ int oldCount = countFiles(tableId);
+
+ // merge two files into one
+ ts.exec("compact -t " + table);
+
+ Thread.sleep(3_000);
+ // compaction should still be running
+
+ log.info("Calling fate print for table = {}", table);
+ ts.exec("fate -print", true, "txid:", true);
+
+ // test filters
+ ts.exec("fate -print -t IN_PROGRESS", true, "txid:", true);
+ ts.exec("fate -print -t NEW", true, "0 transactions", true);
Review Comment:
I'll post this as soon as local tests pass - but there is only 1 compaction running
--
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] milleruntime commented on a diff in pull request #2755: Fix Fate print command and improve ShellServerIT test
Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2755:
URL: https://github.com/apache/accumulo/pull/2755#discussion_r903769401
##########
test/src/main/java/org/apache/accumulo/test/shell/ShellServerIT.java:
##########
@@ -2082,4 +2082,54 @@ public void testSummarySelection() throws Exception {
// check that there are two files, with none having extra summary info
assertMatches(output, "(?sm).*^.*total[:]2[,]\\s+missing[:]0[,]\\s+extra[:]0.*$.*");
}
+
+ @Test
+ public void testFateCommandWithSlowCompaction() throws Exception {
+ final String table = getUniqueNames(1)[0];
+
+ String orgProps = System.getProperty("accumulo.properties");
+
+ System.setProperty("accumulo.properties",
+ "file://" + getCluster().getConfig().getAccumuloPropsFile().getCanonicalPath());
+ // compact
+ ts.exec("createtable " + table);
+
+ // setup SlowIterator to sleep for 10 seconds
+ ts.exec("config -t " + table
+ + " -s table.iterator.majc.slow=1,org.apache.accumulo.test.functional.SlowIterator");
+ ts.exec("config -t " + table + " -s table.iterator.majc.slow.opt.sleepTime=10000");
+
+ String tableId = getTableId(table);
+
+ // make two files
+ ts.exec("insert a1 b c v_a1");
+ ts.exec("insert a2 b c v_a2");
+ ts.exec("flush -w");
+ ts.exec("insert x1 b c v_x1");
+ ts.exec("insert x2 b c v_x2");
+ ts.exec("flush -w");
+ int oldCount = countFiles(tableId);
+
+ // merge two files into one
+ ts.exec("compact -t " + table);
+
+ Thread.sleep(3_000);
+ // compaction should still be running
+
+ log.info("Calling fate print for table = {}", table);
+ ts.exec("fate -print", true, "txid:", true);
+
+ // test filters
+ ts.exec("fate -print -t IN_PROGRESS", true, "txid:", true);
+ ts.exec("fate -print -t NEW", true, "0 transactions", true);
Review Comment:
The first line adds another call to compact the table again.
--
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