This is an automated email from the ASF dual-hosted git repository.
kturner pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/elasticity by this push:
new 7501cb513e fixes a race condition in tablet group watcher (#4048)
7501cb513e is described below
commit 7501cb513e016d1cb896184edc5b4bee0dd732ef
Author: Keith Turner <ktur...@apache.org>
AuthorDate: Mon Dec 11 14:16:20 2023 -0500
fixes a race condition in tablet group watcher (#4048)
There was a race condition between the two thread in tablet group
watcher that caused a tablet to be marked as assigned to a dead tablet
server when that was not true.
This commit fixes that race condition, enables a test that was failing
because of it, and adds the thread id to test logging. The thread id
was useful in confirming this bug so it will probably be useful in the
future.
Below are some log messages from tracking this bug down. The number
after the timestamp is the thread id. Can see thread 47 assigns tablet
1<< and then thread 46 assumes its assigned to a dead tablet server.
However it was a not a dead tserver, it had just started as the test
restarted tservers.
```
2023-12-09T16:04:54,439 47 [manager.Manager] TRACE: [Normal Tablets]
Shutting down all Tservers: false, dependentCount: null Extent: 1<<, state:
UNASSIGNED, goal: HOSTED actions:[NEEDS_LOCATION_UPDATE]
2023-12-09T16:04:54,443 47 [tablet.location] DEBUG: Assigned 1<< to
localhost:37775[10006c85a80001a]
2023-12-09T16:04:54,452 47 [manager.Manager] TRACE: [Normal Tablets]
Shutting down all Tservers: false, dependentCount: null Extent: 1<<, state:
ASSIGNED, goal: HOSTED actions:[NEEDS_LOCATION_UPDATE]
2023-12-09T16:04:55,514 46 [manager.Manager] TRACE: [Normal Tablets]
Shutting down all Tservers: false, dependentCount: null Extent: 1<<, state:
ASSIGNED_TO_DEAD_SERVER, goal: HOSTED actions:[NEEDS_LOCATION_UPDATE]
2023-12-09T16:04:55,515 46 [manager.Manager] DEBUG: 1 assigned to dead
servers:
[TabletMetadata[tableId=1,prevEndRow=<null>,sawPrevEndRow=true,oldPrevEndRow=<null>,sawOldPrevEndRow=false,endRow=<null>,location=Location
[server=localhost:37775[10006c85a80001a],
type=CURRENT],files={},scans=[],loadedFiles={},fetchedCols=[LOCATION, PREV_ROW,
FILES, LAST, DIR, LOGS, SUSPEND, ECOMP, HOSTING_GOAL, HOSTING_REQUESTED, OPID,
SELECTED],extent=1<<,last=Location [server=localhost:37775[10006c85a8 [...]
2023-12-09T16:04:55,536 46 [tablet.location] DEBUG: Suspended 1<< to
localhost:37775 at 54231 ms with 1 walogs
2023-12-09T16:04:55,652 46 [manager.Manager] TRACE: [Normal Tablets]
Shutting down all Tservers: false, dependentCount: null Extent: 1<<, state:
SUSPENDED, goal: HOSTED actions:[NEEDS_LOCATION_UPDATE]
```
---
.../accumulo/manager/TabletGroupWatcher.java | 30 +++++++++++++++++++++-
.../apache/accumulo/test/WriteAfterCloseIT.java | 2 --
test/src/main/resources/log4j2-test.properties | 2 +-
3 files changed, 30 insertions(+), 4 deletions(-)
diff --git
a/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java
b/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java
index 4833e28f57..a217f870b7 100644
---
a/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java
+++
b/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java
@@ -426,7 +426,35 @@ abstract class TabletGroupWatcher extends
AccumuloDaemonThread {
final TableConfiguration tableConf =
manager.getContext().getTableConfiguration(tableId);
- final TabletState state = TabletState.compute(tm,
currentTServers.keySet());
+ TabletState state = TabletState.compute(tm, currentTServers.keySet());
+ if (state == TabletState.ASSIGNED_TO_DEAD_SERVER) {
+ /*
+ * This code exists to deal with a race condition caused by two
threads running in this
+ * class that compute tablets actions. One thread does full scans and
the other reacts to
+ * events and does partial scans. Below is an example of the race
condition this is
+ * handling.
+ *
+ * - TGW Thread 1 : reads the set of tablets servers and its empty
+ *
+ * - TGW Thread 2 : reads the set of tablet servers and its [TS1]
+ *
+ * - TGW Thread 2 : Sees tabletX without a location and assigns it to
TS1
+ *
+ * - TGW Thread 1 : Sees tabletX assigned to TS1 and assumes it's
assigned to a dead tablet
+ * server because its set of live servers is the empty set.
+ *
+ * To deal with this race condition, this code recomputes the tablet
state using the latest
+ * tservers when a tablet is seen assigned to a dead tserver.
+ */
+
+ TabletState newState = TabletState.compute(tm,
manager.tserversSnapshot().getTservers());
+ if (newState != state) {
+ LOG.debug("Tablet state changed when using latest set of tservers {}
{} {}",
+ tm.getExtent(), state, newState);
+ state = newState;
+ }
+ }
+
// This is final because nothing in this method should change the goal.
All computation of the
// goal should be done in TabletGoalState.compute() so that all parts of
the Accumulo code
// will compute a consistent goal.
diff --git a/test/src/main/java/org/apache/accumulo/test/WriteAfterCloseIT.java
b/test/src/main/java/org/apache/accumulo/test/WriteAfterCloseIT.java
index c77b57b2cf..34aa1de519 100644
--- a/test/src/main/java/org/apache/accumulo/test/WriteAfterCloseIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/WriteAfterCloseIT.java
@@ -50,7 +50,6 @@ import org.apache.accumulo.minicluster.ServerType;
import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.RawLocalFileSystem;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
public class WriteAfterCloseIT extends AccumuloClusterHarness {
@@ -108,7 +107,6 @@ public class WriteAfterCloseIT extends
AccumuloClusterHarness {
}
@Test
- @Disabled // ELASTICITY_TODO
public void testWriteAfterCloseKillTservers() throws Exception {
runTest(TimeType.MILLIS, true, 0, false);
}
diff --git a/test/src/main/resources/log4j2-test.properties
b/test/src/main/resources/log4j2-test.properties
index f0d7d93212..70276bda59 100644
--- a/test/src/main/resources/log4j2-test.properties
+++ b/test/src/main/resources/log4j2-test.properties
@@ -25,7 +25,7 @@ appender.console.type = Console
appender.console.name = STDOUT
appender.console.target = SYSTEM_OUT
appender.console.layout.type = PatternLayout
-appender.console.layout.pattern = %d{ISO8601} [%c{2}] %-5p: %m%n
+appender.console.layout.pattern = %d{ISO8601} %T [%c{2}] %-5p: %m%n
logger.01.name = org.apache.accumulo.core
logger.01.level = debug
