You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "dlmarion (via GitHub)" <gi...@apache.org> on 2023/05/30 14:54:12 UTC
[GitHub] [accumulo] dlmarion opened a new pull request, #3439: Removed SplitScanner, moved into TabletGroupWatcher
dlmarion opened a new pull request, #3439:
URL: https://github.com/apache/accumulo/pull/3439
(no comment)
--
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 #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1211768892
##########
server/manager/src/test/java/org/apache/accumulo/manager/split/SplitterTest.java:
##########
@@ -65,23 +65,23 @@ public void testShouldInspect() {
expect(tabletMeta2.getExtent()).andReturn(ke2).anyTimes();
replay(tabletMeta2);
- assertTrue(splitter.shouldInspect(tabletMeta1));
Review Comment:
Addressed in cfced0b
##########
server/manager/src/main/java/org/apache/accumulo/manager/split/Splitter.java:
##########
@@ -197,6 +182,10 @@ public boolean shouldInspect(TabletMetadata tablet) {
if (hashCode != null) {
if (hashCode.equals(caclulateFilesHash(tablet))) {
return false;
+ } else {
+ // We know that the list of files for this tablet have changed
+ // so we can remove it from the set of unsplittable tablets.
+ unsplittable.invalidate(tablet.getExtent());
Review Comment:
Addressed in cfced0b
--
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] keith-turner commented on a diff in pull request #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1211666752
##########
server/manager/src/main/java/org/apache/accumulo/manager/split/Splitter.java:
##########
@@ -112,10 +102,7 @@ private static int weigh(KeyExtent keyExtent) {
return size;
}
- public Splitter(ServerContext context, Ample.DataLevel level, Manager manager) {
- this.context = context;
- this.level = level;
- this.manager = manager;
+ public Splitter(ServerContext context) {
this.splitExecutor = context.threadPools().createExecutorService(context.getConfiguration(),
Property.MANAGER_SPLIT_WORKER_THREADS, true);
this.scanExecutor =
Review Comment:
This thread pool may be able to be removed
##########
server/manager/src/main/java/org/apache/accumulo/manager/split/Splitter.java:
##########
@@ -197,6 +182,10 @@ public boolean shouldInspect(TabletMetadata tablet) {
if (hashCode != null) {
if (hashCode.equals(caclulateFilesHash(tablet))) {
return false;
+ } else {
+ // We know that the list of files for this tablet have changed
+ // so we can remove it from the set of unsplittable tablets.
+ unsplittable.invalidate(tablet.getExtent());
Review Comment:
This is a nice fix. Would be nice to add a unit test for this case.
##########
server/manager/src/test/java/org/apache/accumulo/manager/split/SplitterTest.java:
##########
@@ -65,23 +65,23 @@ public void testShouldInspect() {
expect(tabletMeta2.getExtent()).andReturn(ke2).anyTimes();
replay(tabletMeta2);
- assertTrue(splitter.shouldInspect(tabletMeta1));
Review Comment:
The method name for this test has shouldInspect in it, could rename the test method.
--
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] keith-turner commented on a diff in pull request #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1210867136
##########
test/src/main/java/org/apache/accumulo/test/functional/SplitIT.java:
##########
@@ -140,6 +140,8 @@ public void tabletShouldSplit() throws Exception {
VerifyParams params = new VerifyParams(getClientProps(), table, 100_000);
TestIngest.ingest(c, params);
VerifyIngest.verifyIngest(c, params);
+ // ELASTICITY_TODO: Sum of file sizes is zero in
+ // TabletManagementIterator.shouldReturnDueToSplit, so split never happens
Review Comment:
Opened #3442 with the 2nd fix found while looking into this
--
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] keith-turner commented on a diff in pull request #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1210901109
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java:
##########
@@ -214,8 +214,12 @@ private static Map<TableId,MergeInfo> parseMerges(final String merges) {
private static boolean shouldReturnDueToSplit(final TabletMetadata tm,
final long splitThreshold) {
- return tm.getFilesMap().values().stream().map(DataFileValue::getSize)
- .collect(Collectors.summarizingLong(Long::longValue)).getSum() > splitThreshold;
+ final long sumOfFileSizes =
Review Comment:
When there is an operation id set we can ignore the tablet for automatic split.
```suggestion
if (tm.getOperationId() != null) {
// tablet may be currently splitting or merging, can ignore it until that finishes
return false;
}
final long sumOfFileSizes =
```
--
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] keith-turner commented on a diff in pull request #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1211770387
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java:
##########
@@ -214,8 +214,12 @@ private static Map<TableId,MergeInfo> parseMerges(final String merges) {
private static boolean shouldReturnDueToSplit(final TabletMetadata tm,
final long splitThreshold) {
- return tm.getFilesMap().values().stream().map(DataFileValue::getSize)
- .collect(Collectors.summarizingLong(Long::longValue)).getSum() > splitThreshold;
+ final long sumOfFileSizes =
Review Comment:
> It might need to be returned so that it can be unassigned?
Yeah maybe for merge. For the case where a tablet has an opid, it should only have an opid if there is no location. Opid and location should be mutually exclusive like current and 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
[GitHub] [accumulo] keith-turner commented on a diff in pull request #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1211783305
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java:
##########
@@ -214,8 +214,12 @@ private static Map<TableId,MergeInfo> parseMerges(final String merges) {
private static boolean shouldReturnDueToSplit(final TabletMetadata tm,
final long splitThreshold) {
- return tm.getFilesMap().values().stream().map(DataFileValue::getSize)
- .collect(Collectors.summarizingLong(Long::longValue)).getSum() > splitThreshold;
+ final long sumOfFileSizes =
Review Comment:
That sounds good, I will open a follow on 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] dlmarion merged pull request #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion merged PR #3439:
URL: https://github.com/apache/accumulo/pull/3439
--
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] keith-turner commented on a diff in pull request #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1210752090
##########
test/src/main/java/org/apache/accumulo/test/functional/SplitIT.java:
##########
@@ -140,6 +140,8 @@ public void tabletShouldSplit() throws Exception {
VerifyParams params = new VerifyParams(getClientProps(), table, 100_000);
TestIngest.ingest(c, params);
VerifyIngest.verifyIngest(c, params);
+ // ELASTICITY_TODO: Sum of file sizes is zero in
+ // TabletManagementIterator.shouldReturnDueToSplit, so split never happens
Review Comment:
I am looking into this locally. Added the following to the TabletManagementIterator.
```java
try {
computeTabletManagementActions(tm, actions);
}catch(IllegalStateException ise) {
LOG.info("decodedRow.size() : {}", decodedRow.size());
decodedRow.forEach((k2,v2)->LOG.info("{} {}",k2,v2));
throw ise;
}
```
When there is a failure I am seeing a tablet that only has a future location. Still looking into the cause of that.
We may want to add some validation of the TabletManagementIterator.seek() to check if all columns or the expected columns are being seeked. I added some debug to see if something was just seeking the future column and that was not the case.
I also locally merged the latest elasticity changes into this branch.
--
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 #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1211770556
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java:
##########
@@ -214,8 +214,12 @@ private static Map<TableId,MergeInfo> parseMerges(final String merges) {
private static boolean shouldReturnDueToSplit(final TabletMetadata tm,
final long splitThreshold) {
- return tm.getFilesMap().values().stream().map(DataFileValue::getSize)
- .collect(Collectors.summarizingLong(Long::longValue)).getSum() > splitThreshold;
+ final long sumOfFileSizes =
Review Comment:
I added this check in cfced0b, but the iterator still returns location updates when opid != null, for 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 #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1211748798
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java:
##########
@@ -214,8 +214,12 @@ private static Map<TableId,MergeInfo> parseMerges(final String merges) {
private static boolean shouldReturnDueToSplit(final TabletMetadata tm,
final long splitThreshold) {
- return tm.getFilesMap().values().stream().map(DataFileValue::getSize)
- .collect(Collectors.summarizingLong(Long::longValue)).getSum() > splitThreshold;
+ final long sumOfFileSizes =
Review Comment:
> So for the location update case it does seem like there is no need to return it. Merge is going to change a lot, so not sure about it at the moment.
It might need to be returned so that it can be unassigned?
--
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] keith-turner commented on a diff in pull request #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1211792071
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java:
##########
@@ -214,8 +214,12 @@ private static Map<TableId,MergeInfo> parseMerges(final String merges) {
private static boolean shouldReturnDueToSplit(final TabletMetadata tm,
final long splitThreshold) {
- return tm.getFilesMap().values().stream().map(DataFileValue::getSize)
- .collect(Collectors.summarizingLong(Long::longValue)).getSum() > splitThreshold;
+ final long sumOfFileSizes =
Review Comment:
Created #3443
--
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 #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1211597563
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java:
##########
@@ -214,8 +214,12 @@ private static Map<TableId,MergeInfo> parseMerges(final String merges) {
private static boolean shouldReturnDueToSplit(final TabletMetadata tm,
final long splitThreshold) {
- return tm.getFilesMap().values().stream().map(DataFileValue::getSize)
- .collect(Collectors.summarizingLong(Long::longValue)).getSum() > splitThreshold;
+ final long sumOfFileSizes =
Review Comment:
You mention for `split`, but do you think that TabletManagementIterator should not return the TabletMetadata for any condition when there is an operationId set?
--
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] keith-turner commented on a diff in pull request #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1210921238
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java:
##########
@@ -214,8 +214,12 @@ private static Map<TableId,MergeInfo> parseMerges(final String merges) {
private static boolean shouldReturnDueToSplit(final TabletMetadata tm,
final long splitThreshold) {
- return tm.getFilesMap().values().stream().map(DataFileValue::getSize)
- .collect(Collectors.summarizingLong(Long::longValue)).getSum() > splitThreshold;
+ final long sumOfFileSizes =
Review Comment:
In addition to this check early on in the tserver, I added a check that is as late as possible in the manager. I added that in c8d39de73d7984a490c3693aa9760c96546b1bb2 w/o doing a PR. Its such a small change and this PR does not modify that file. Let me know if you see any problems w/ that little 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] dlmarion commented on a diff in pull request #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1210417415
##########
test/src/main/java/org/apache/accumulo/test/functional/SplitIT.java:
##########
@@ -140,6 +140,8 @@ public void tabletShouldSplit() throws Exception {
VerifyParams params = new VerifyParams(getClientProps(), table, 100_000);
TestIngest.ingest(c, params);
VerifyIngest.verifyIngest(c, params);
+ // ELASTICITY_TODO: Sum of file sizes is zero in
+ // TabletManagementIterator.shouldReturnDueToSplit, so split never happens
Review Comment:
@keith-turner - this comment is no longer relevant. `scanner.fetchColumnFamily(DataFileColumnFamily.NAME)` was missing from `TabletMetadataIterator.tabletShouldSplit`. However, this test is causing some interesting log messages and the test does not pass. For example in the Manager log:
```
2023-05-30T14:48:28,283 [manager.TabletGroupWatcher] DEBUG: 1<< may need splitting.
2023-05-30T14:48:28,283 [manager.TabletGroupWatcher] DEBUG: submitting tablet 1<< for split
...
2023-05-30T14:48:28,625 [manager.EventCoordinator] INFO : Unassignment requested 1<<
2023-05-30T14:48:28,630 [fate.Fate] INFO : Seeding FATE[20d8a7db9bc013e4] System initiated split of tablet 1<< into 23 splits
2023-05-30T14:48:28,632 [manager.Manager] DEBUG: Finished gathering information from 2 of 2 servers in 0.01 seconds
2023-05-30T14:48:28,667 [split.PreSplit] DEBUG: FATE[20d8a7db9bc013e4] Failed to set operation id. extent:1<< location:Location [server=ip-10-113-15-120.evoforge.org:45969[100009be7ac0006], type=CURRENT] opid:null
2023-05-30T14:48:28,667 [manager.EventCoordinator] INFO : Unassignment requested 1<<
...
2023-05-30T14:48:28,734 [manager.TabletGroupWatcher] DEBUG: 1<< may need splitting.
2023-05-30T14:48:28,734 [manager.TabletGroupWatcher] DEBUG: 1<< is not splittable.
```
and in the TabletServer log:
```
2023-05-30T14:48:40,076 [scan.LookupTask] WARN : exception while doing multi-scan
java.lang.IllegalStateException: No prev endrow seen. tableId: 1 endrow: row_00000124�
at org.apache.accumulo.core.metadata.schema.TabletMetadata.getPrevEndRow(TabletMetadata.java:263) ~[classes/:?]
at org.apache.accumulo.core.metadata.schema.TabletMetadata.getExtent(TabletMetadata.java:251) ~[classes/:?]
at org.apache.accumulo.server.manager.state.TabletManagementIterator.shouldReturnDueToLocation(TabletManagementIterator.java:228) ~[classes/:?]
at org.apache.accumulo.server.manager.state.TabletManagementIterator.computeTabletManagementActions(TabletManagementIterator.java:409) ~[classes/:?]
at org.apache.accumulo.server.manager.state.TabletManagementIterator.consume(TabletManagementIterator.java:368) ~[classes/:?]
```
--
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] keith-turner commented on a diff in pull request #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1211663540
##########
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java:
##########
@@ -214,8 +214,12 @@ private static Map<TableId,MergeInfo> parseMerges(final String merges) {
private static boolean shouldReturnDueToSplit(final TabletMetadata tm,
final long splitThreshold) {
- return tm.getFilesMap().values().stream().map(DataFileValue::getSize)
- .collect(Collectors.summarizingLong(Long::longValue)).getSum() > splitThreshold;
+ final long sumOfFileSizes =
Review Comment:
There should not be a need to assign a tablet with an op id. So for the location update case it does seem like there is no need to return it. Merge is going to change a lot, so not sure about it at the moment.
Also a tablet with an op id should never have a location. Maybe we could add another check for the bad state case.
--
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 #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1211598260
##########
test/src/main/java/org/apache/accumulo/test/functional/SplitIT.java:
##########
@@ -140,6 +140,8 @@ public void tabletShouldSplit() throws Exception {
VerifyParams params = new VerifyParams(getClientProps(), table, 100_000);
TestIngest.ingest(c, params);
VerifyIngest.verifyIngest(c, params);
+ // ELASTICITY_TODO: Sum of file sizes is zero in
+ // TabletManagementIterator.shouldReturnDueToSplit, so split never happens
Review Comment:
I merged your changes and this test passes. Thanks for the assist!
--
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 #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1211769310
##########
server/manager/src/main/java/org/apache/accumulo/manager/split/Splitter.java:
##########
@@ -112,10 +102,7 @@ private static int weigh(KeyExtent keyExtent) {
return size;
}
- public Splitter(ServerContext context, Ample.DataLevel level, Manager manager) {
- this.context = context;
- this.level = level;
- this.manager = manager;
+ public Splitter(ServerContext context) {
this.splitExecutor = context.threadPools().createExecutorService(context.getConfiguration(),
Property.MANAGER_SPLIT_WORKER_THREADS, true);
this.scanExecutor =
Review Comment:
Addressed in cfced0b
--
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] keith-turner commented on a diff in pull request #3439: Removed SplitScanner, moved into TabletGroupWatcher
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #3439:
URL: https://github.com/apache/accumulo/pull/3439#discussion_r1210847240
##########
test/src/main/java/org/apache/accumulo/test/functional/SplitIT.java:
##########
@@ -140,6 +140,8 @@ public void tabletShouldSplit() throws Exception {
VerifyParams params = new VerifyParams(getClientProps(), table, 100_000);
TestIngest.ingest(c, params);
VerifyIngest.verifyIngest(c, params);
+ // ELASTICITY_TODO: Sum of file sizes is zero in
+ // TabletManagementIterator.shouldReturnDueToSplit, so split never happens
Review Comment:
Tracked down two problems, have submitted a fix for one : #3441
I added a lot of debugging and finally figured out what was happening. I was seeing the tserver return row `row_00000124%ff;` where `%ff;` is binary. In the manager it was processing `row_00000124%ef;%bf;%bd;` where `%ef;%bf;%bd;` is binary. This was caused by data going through String which #3441 fixes. The row the manager was processing did not actually exists. It would write a future location for this non-existent tablet and that would cause the tablet w/o a prev row. I am going to submit another little fix where a tablet can not write a future if the tablet does not exists, that should not be able to happen.
--
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