This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push:
new 0cdf6dd2de Revert "Remove redundant LogEntry value (#3997)"
0cdf6dd2de is described below
commit 0cdf6dd2de0ca382c3881169bfe90c3bbd4367b3
Author: Christopher Tubbs <ctubb...@apache.org>
AuthorDate: Wed Dec 13 15:47:31 2023 -0500
Revert "Remove redundant LogEntry value (#3997)"
This issue caused #4065 and needs to be reverted and redone. The
explanation is the misunderstanding of the LogEntry schema at
https://github.com/apache/accumulo/issues/4058#issuecomment-1854454775
This reverts commit 6cfa1b2ea72816ebbb30d462843d223a126947c9.
---
.../accumulo/core/tabletserver/log/LogEntry.java | 61 ++++++++++++----------
.../core/metadata/schema/TabletMetadataTest.java | 11 ++--
.../core/tabletserver/log/LogEntryTest.java | 11 ++--
.../org/apache/accumulo/server/fs/VolumeUtil.java | 10 ++--
.../server/manager/state/ZooTabletStateStore.java | 4 +-
.../server/metadata/TabletMutatorBase.java | 2 +-
.../accumulo/server/util/ListVolumesUsed.java | 2 +-
.../apache/accumulo/server/fs/VolumeUtilTest.java | 6 +--
.../org/apache/accumulo/tserver/TabletServer.java | 2 +-
.../org/apache/accumulo/tserver/tablet/Tablet.java | 2 +-
.../test/MissingWalHeaderCompletesRecoveryIT.java | 6 ++-
.../apache/accumulo/test/manager/MergeStateIT.java | 3 +-
12 files changed, 66 insertions(+), 54 deletions(-)
diff --git
a/core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
b/core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
index ecdb20381a..9c592b2260 100644
--- a/core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
+++ b/core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java
@@ -23,25 +23,22 @@ import java.util.Objects;
import java.util.UUID;
import org.apache.accumulo.core.data.Key;
-import org.apache.accumulo.core.data.Mutation;
import org.apache.accumulo.core.data.Value;
-import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LogColumnFamily;
import org.apache.hadoop.io.Text;
-import com.google.common.base.Preconditions;
import com.google.common.net.HostAndPort;
public class LogEntry {
- private final String logReference;
+ private final String filePath;
- public LogEntry(String logReference) {
- validateLogReference(logReference);
- this.logReference = logReference;
+ public LogEntry(String filePath) {
+ validateFilePath(filePath);
+ this.filePath = filePath;
}
- public String getLogReference() {
- return this.logReference;
+ public String getFilePath() {
+ return this.filePath;
}
/**
@@ -49,15 +46,15 @@ public class LogEntry {
* (host:port) followed by a UUID as the file name. For example,
* localhost:1234/927ba659-d109-4bce-b0a5-bcbbcb9942a2 is a valid file path.
*
- * @param logReference path to validate
+ * @param filePath path to validate
* @throws IllegalArgumentException if the filepath is invalid
*/
- private static void validateLogReference(String logReference) {
- String[] parts = logReference.split("/");
+ private static void validateFilePath(String filePath) {
+ String[] parts = filePath.split("/");
if (parts.length < 2) {
throw new IllegalArgumentException(
- "Invalid logReference format. The path should at least contain
tserver/UUID.");
+ "Invalid filePath format. The path should at least contain
tserver/UUID.");
}
String tserverPart = parts[parts.length - 2];
@@ -67,8 +64,8 @@ public class LogEntry {
HostAndPort.fromString(tserverPart);
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException(
- "Invalid tserver format in logReference. Expected format: host:port.
Found '"
- + tserverPart + "'");
+ "Invalid tserver format in filePath. Expected format: host:port.
Found '" + tserverPart
+ + "'");
}
try {
@@ -79,17 +76,17 @@ public class LogEntry {
}
/**
- * Add LogEntry information to the provided mutation.
+ * Make a copy of this LogEntry but replace the file path.
*
- * @param mutation the mutation to update
+ * @param filePath path to use
*/
- public void addToMutation(Mutation mutation) {
-
mutation.at().family(LogColumnFamily.NAME).qualifier(getColumnQualifier()).put(new
Value());
+ public LogEntry switchFile(String filePath) {
+ return new LogEntry(filePath);
}
@Override
public String toString() {
- return logReference;
+ return filePath;
}
@Override
@@ -101,29 +98,35 @@ public class LogEntry {
return false;
}
LogEntry logEntry = (LogEntry) other;
- return this.logReference.equals(logEntry.logReference);
+ return this.filePath.equals(logEntry.filePath);
}
@Override
public int hashCode() {
- return Objects.hash(logReference);
+ return Objects.hash(filePath);
}
public static LogEntry fromMetaWalEntry(Entry<Key,Value> entry) {
- String qualifier = entry.getKey().getColumnQualifier().toString();
- String[] parts = qualifier.split("/", 2);
- Preconditions.checkArgument(parts.length == 2 && parts[0].equals("-"),
- "Malformed write-ahead log %s", qualifier);
- return new LogEntry(parts[1]);
+ final Value value = entry.getValue();
+
+ String filePath = value.toString();
+
+ validateFilePath(filePath);
+
+ return new LogEntry(filePath);
}
public String getUniqueID() {
- String[] parts = logReference.split("/");
+ String[] parts = filePath.split("/");
return parts[parts.length - 1];
}
public Text getColumnQualifier() {
- return new Text("-/" + logReference);
+ return new Text("-/" + filePath);
+ }
+
+ public Value getValue() {
+ return new Value(filePath);
}
}
diff --git
a/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java
b/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java
index a4cb1b583c..d547b8596d 100644
---
a/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java
@@ -59,6 +59,7 @@ import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.Cu
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.DataFileColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.FutureLocationColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LastLocationColumnFamily;
+import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LogColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ScanFileColumnFamily;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SuspendLocationColumn;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.TabletColumnFamily;
@@ -103,9 +104,11 @@ public class TabletMetadataTest {
mutation.at().family(LastLocationColumnFamily.NAME).qualifier("s000").put("server2:8555");
LogEntry le1 = new LogEntry("localhost:8020/" + UUID.randomUUID());
- le1.addToMutation(mutation);
+
mutation.at().family(LogColumnFamily.NAME).qualifier(le1.getColumnQualifier())
+ .put(le1.getValue());
LogEntry le2 = new LogEntry("localhost:8020/" + UUID.randomUUID());
- le2.addToMutation(mutation);
+
mutation.at().family(LogColumnFamily.NAME).qualifier(le2.getColumnQualifier())
+ .put(le2.getValue());
StoredTabletFile sf1 = StoredTabletFile.of(new
Path("hdfs://nn1/acc/tables/1/t-0001/sf1.rf"));
StoredTabletFile sf2 = StoredTabletFile.of(new
Path("hdfs://nn1/acc/tables/1/t-0001/sf2.rf"));
@@ -137,8 +140,8 @@ public class TabletMetadataTest {
assertEquals(HostAndPort.fromParts("server2", 8555),
tm.getLast().getHostAndPort());
assertEquals("s000", tm.getLast().getSession());
assertEquals(LocationType.LAST, tm.getLast().getType());
- assertEquals(Set.of(le1.getColumnQualifier(), le2.getColumnQualifier()),
-
tm.getLogs().stream().map(LogEntry::getColumnQualifier).collect(toSet()));
+ assertEquals(Set.of(le1.getValue(), le2.getValue()),
+ tm.getLogs().stream().map(LogEntry::getValue).collect(toSet()));
assertEquals(extent.prevEndRow(), tm.getPrevEndRow());
assertEquals(extent.tableId(), tm.getTableId());
assertTrue(tm.sawPrevEndRow());
diff --git
a/core/src/test/java/org/apache/accumulo/core/tabletserver/log/LogEntryTest.java
b/core/src/test/java/org/apache/accumulo/core/tabletserver/log/LogEntryTest.java
index 9fd5c3f1d2..302021121e 100644
---
a/core/src/test/java/org/apache/accumulo/core/tabletserver/log/LogEntryTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/tabletserver/log/LogEntryTest.java
@@ -52,18 +52,20 @@ public class LogEntryTest {
// test from constructor
LogEntry one = new LogEntry(validFilename);
assertEquals(validFilename, one.toString());
- assertEquals(validFilename, one.getLogReference());
+ assertEquals(validFilename, one.getFilePath());
assertEquals(new Text("-/" + validFilename), one.getColumnQualifier());
assertEquals(validUUID.toString(), one.getUniqueID());
+ assertEquals(new Value(validFilename), one.getValue());
// test from metadata entry
LogEntry two = LogEntry.fromMetaWalEntry(new
AbstractMap.SimpleImmutableEntry<>(
- new Key(new Text("1<"), new Text("log"), one.getColumnQualifier()),
new Value("unused")));
+ new Key(new Text("1<"), new Text("log"), one.getColumnQualifier()),
one.getValue()));
assertNotSame(one, two);
assertEquals(one.toString(), two.toString());
- assertEquals(one.getLogReference(), two.getLogReference());
+ assertEquals(one.getFilePath(), two.getFilePath());
assertEquals(one.getColumnQualifier(), two.getColumnQualifier());
assertEquals(one.getUniqueID(), two.getUniqueID());
+ assertEquals(one.getValue(), two.getValue());
assertEquals(one, two);
}
@@ -74,9 +76,10 @@ public class LogEntryTest {
assertNotSame(one, two);
assertEquals(one.toString(), two.toString());
- assertEquals(one.getLogReference(), two.getLogReference());
+ assertEquals(one.getFilePath(), two.getFilePath());
assertEquals(one.getColumnQualifier(), two.getColumnQualifier());
assertEquals(one.getUniqueID(), two.getUniqueID());
+ assertEquals(one.getValue(), two.getValue());
assertEquals(one, two);
assertEquals(one, one);
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java
b/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java
index 29f470ebe6..98920aa2ea 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/fs/VolumeUtil.java
@@ -88,14 +88,14 @@ public class VolumeUtil {
}
protected static LogEntry switchVolumes(LogEntry le, List<Pair<Path,Path>>
replacements) {
- Path switchedPath = switchVolume(new Path(le.getLogReference()),
FileType.WAL, replacements);
+ Path switchedPath = switchVolume(new Path(le.getFilePath()), FileType.WAL,
replacements);
String switchedString;
int numSwitched = 0;
if (switchedPath != null) {
switchedString = switchedPath.toString();
numSwitched++;
} else {
- switchedString = le.getLogReference();
+ switchedString = le.getFilePath();
}
if (numSwitched == 0) {
@@ -103,7 +103,7 @@ public class VolumeUtil {
return null;
}
- LogEntry newLogEntry = new LogEntry(switchedString);
+ LogEntry newLogEntry = le.switchFile(switchedString);
log.trace("Switched {} to {}", le, newLogEntry);
@@ -155,8 +155,8 @@ public class VolumeUtil {
logsToRemove.add(logEntry);
logsToAdd.add(switchedLogEntry);
ret.logEntries.add(switchedLogEntry);
- log.debug("Replacing volume {} : {} -> {}", extent,
logEntry.getLogReference(),
- switchedLogEntry.getLogReference());
+ log.debug("Replacing volume {} : {} -> {}", extent,
logEntry.getFilePath(),
+ switchedLogEntry.getFilePath());
} else {
ret.logEntries.add(logEntry);
}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/manager/state/ZooTabletStateStore.java
b/server/base/src/main/java/org/apache/accumulo/server/manager/state/ZooTabletStateStore.java
index c628766b91..c6bc4bef39 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/manager/state/ZooTabletStateStore.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/manager/state/ZooTabletStateStore.java
@@ -98,8 +98,8 @@ class ZooTabletStateStore implements TabletStateStore {
List<Collection<String>> logs = new ArrayList<>();
rootMeta.getLogs().forEach(logEntry -> {
- logs.add(Collections.singleton(logEntry.getLogReference()));
- log.debug("root tablet log {}", logEntry.getLogReference());
+ logs.add(Collections.singleton(logEntry.getFilePath()));
+ log.debug("root tablet log {}", logEntry.getFilePath());
});
return new TabletLocationState(RootTable.EXTENT, futureSession,
currentSession,
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java
b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java
index 4f7f9c230c..73776db9b9 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java
@@ -175,7 +175,7 @@ public abstract class TabletMutatorBase implements
Ample.TabletMutator {
@Override
public Ample.TabletMutator putWal(LogEntry logEntry) {
Preconditions.checkState(updatesEnabled, "Cannot make updates after
calling mutate.");
- logEntry.addToMutation(mutation);
+ mutation.put(LogColumnFamily.NAME, logEntry.getColumnQualifier(),
logEntry.getValue());
return this;
}
diff --git
a/server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java
b/server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java
index a3366facd7..2055d793d4 100644
---
a/server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java
+++
b/server/base/src/main/java/org/apache/accumulo/server/util/ListVolumesUsed.java
@@ -55,7 +55,7 @@ public class ListVolumesUsed {
}
private static void getLogURIs(TreeSet<String> volumes, LogEntry logEntry) {
- volumes.add(getLogURI(logEntry.getLogReference()));
+ volumes.add(getLogURI(logEntry.getFilePath()));
}
private static void listTable(Ample.DataLevel level, ServerContext context)
throws Exception {
diff --git
a/server/base/src/test/java/org/apache/accumulo/server/fs/VolumeUtilTest.java
b/server/base/src/test/java/org/apache/accumulo/server/fs/VolumeUtilTest.java
index 80db5f6906..b6ac99845d 100644
---
a/server/base/src/test/java/org/apache/accumulo/server/fs/VolumeUtilTest.java
+++
b/server/base/src/test/java/org/apache/accumulo/server/fs/VolumeUtilTest.java
@@ -181,16 +181,16 @@ public class VolumeUtilTest {
String fileName = "hdfs://nn1/accumulo/wal/localhost+9997/" + walUUID;
LogEntry le = new LogEntry(fileName);
LogEntry fixedVolume = VolumeUtil.switchVolumes(le, replacements);
- assertEquals("viewfs:/a/accumulo/wal/localhost+9997/" + walUUID,
fixedVolume.getLogReference());
+ assertEquals("viewfs:/a/accumulo/wal/localhost+9997/" + walUUID,
fixedVolume.getFilePath());
fileName = "hdfs://nn1:9000/accumulo/wal/localhost+9997/" + walUUID;
le = new LogEntry(fileName);
fixedVolume = VolumeUtil.switchVolumes(le, replacements);
- assertEquals("viewfs:/a/accumulo/wal/localhost+9997/" + walUUID,
fixedVolume.getLogReference());
+ assertEquals("viewfs:/a/accumulo/wal/localhost+9997/" + walUUID,
fixedVolume.getFilePath());
fileName = "hdfs://nn2/accumulo/wal/localhost+9997/" + walUUID;
le = new LogEntry(fileName);
fixedVolume = VolumeUtil.switchVolumes(le, replacements);
- assertEquals("viewfs:/b/accumulo/wal/localhost+9997/" + walUUID,
fixedVolume.getLogReference());
+ assertEquals("viewfs:/b/accumulo/wal/localhost+9997/" + walUUID,
fixedVolume.getFilePath());
}
}
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
index 2a205f4cf4..5df7c75169 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
@@ -1103,7 +1103,7 @@ public class TabletServer extends AbstractServer
implements TabletHostingServer
List<Path> recoveryDirs = new ArrayList<>();
for (LogEntry entry : logEntries) {
Path recovery = null;
- Path finished = RecoveryPath.getRecoveryPath(new
Path(entry.getLogReference()));
+ Path finished = RecoveryPath.getRecoveryPath(new
Path(entry.getFilePath()));
finished = SortedLogState.getFinishedMarkerPath(finished);
TabletServer.log.debug("Looking for " + finished);
if (fs.exists(finished)) {
diff --git
a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
index 65db951634..f530058876 100644
---
a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
+++
b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
@@ -370,7 +370,7 @@ public class Tablet extends TabletBase {
currentLogs = new HashSet<>();
for (LogEntry logEntry : logEntries) {
currentLogs.add(new DfsLogger(tabletServer.getContext(),
tabletServer.getServerConfig(),
- logEntry.getLogReference(),
logEntry.getColumnQualifier().toString()));
+ logEntry.getFilePath(), logEntry.getColumnQualifier().toString()));
}
rebuildReferencedLogs();
diff --git
a/test/src/main/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
b/test/src/main/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
index f6711ff223..983f9217d8 100644
---
a/test/src/main/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
+++
b/test/src/main/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
@@ -146,7 +146,8 @@ public class MissingWalHeaderCompletesRecoveryIT extends
ConfigurableMacBase {
Text row = TabletsSection.encodeRow(tableId, null);
Mutation m = new Mutation(row);
- logEntry.addToMutation(m);
+ m.put(TabletsSection.LogColumnFamily.NAME, logEntry.getColumnQualifier(),
+ logEntry.getValue());
try (BatchWriter bw = client.createBatchWriter(MetadataTable.NAME)) {
bw.addMutation(m);
@@ -204,7 +205,8 @@ public class MissingWalHeaderCompletesRecoveryIT extends
ConfigurableMacBase {
Text row = TabletsSection.encodeRow(tableId, null);
Mutation m = new Mutation(row);
- logEntry.addToMutation(m);
+ m.put(TabletsSection.LogColumnFamily.NAME, logEntry.getColumnQualifier(),
+ logEntry.getValue());
try (BatchWriter bw = client.createBatchWriter(MetadataTable.NAME)) {
bw.addMutation(m);
diff --git
a/test/src/main/java/org/apache/accumulo/test/manager/MergeStateIT.java
b/test/src/main/java/org/apache/accumulo/test/manager/MergeStateIT.java
index 40c2e07b5f..699a8f7eaf 100644
--- a/test/src/main/java/org/apache/accumulo/test/manager/MergeStateIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/manager/MergeStateIT.java
@@ -204,7 +204,8 @@ public class MergeStateIT extends ConfigurableMacBase {
KeyExtent ke = new KeyExtent(tableId, new Text("t"), new Text("p"));
m = new Mutation(ke.toMetaRow());
LogEntry logEntry = new LogEntry("localhost:1234/" + UUID.randomUUID());
- logEntry.addToMutation(m);
+
m.at().family(LogColumnFamily.NAME).qualifier(logEntry.getColumnQualifier())
+ .put(logEntry.getValue());
update(accumuloClient, m);
// Verify state is still WAITING_FOR_OFFLINE
