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 6cfa1b2ea7 Remove redundant LogEntry value (#3997)
6cfa1b2ea7 is described below
commit 6cfa1b2ea72816ebbb30d462843d223a126947c9
Author: rsingh433 <74160026+rsingh...@users.noreply.github.com>
AuthorDate: Tue Dec 12 13:46:38 2023 -0500
Remove redundant LogEntry value (#3997)
Remove the `LogEntry.getValue()` method and use the column qualifier
for the log reference, without the redundant storage in the Value
* Remove `getValue()` method and use empty `new Value()`
* Rename "filePath" variables to "logReference"
* Create addToMutation method to remove boilerplate for storing
the LogEntry in the metadata
* Strictly validate the metadata log entries' column qualifier
---------
Co-authored-by: Christopher Tubbs <ctubb...@apache.org>
---
.../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, 54 insertions(+), 66 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 9c592b2260..ecdb20381a 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,22 +23,25 @@ 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 filePath;
+ private final String logReference;
- public LogEntry(String filePath) {
- validateFilePath(filePath);
- this.filePath = filePath;
+ public LogEntry(String logReference) {
+ validateLogReference(logReference);
+ this.logReference = logReference;
}
- public String getFilePath() {
- return this.filePath;
+ public String getLogReference() {
+ return this.logReference;
}
/**
@@ -46,15 +49,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 filePath path to validate
+ * @param logReference path to validate
* @throws IllegalArgumentException if the filepath is invalid
*/
- private static void validateFilePath(String filePath) {
- String[] parts = filePath.split("/");
+ private static void validateLogReference(String logReference) {
+ String[] parts = logReference.split("/");
if (parts.length < 2) {
throw new IllegalArgumentException(
- "Invalid filePath format. The path should at least contain
tserver/UUID.");
+ "Invalid logReference format. The path should at least contain
tserver/UUID.");
}
String tserverPart = parts[parts.length - 2];
@@ -64,8 +67,8 @@ public class LogEntry {
HostAndPort.fromString(tserverPart);
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException(
- "Invalid tserver format in filePath. Expected format: host:port.
Found '" + tserverPart
- + "'");
+ "Invalid tserver format in logReference. Expected format: host:port.
Found '"
+ + tserverPart + "'");
}
try {
@@ -76,17 +79,17 @@ public class LogEntry {
}
/**
- * Make a copy of this LogEntry but replace the file path.
+ * Add LogEntry information to the provided mutation.
*
- * @param filePath path to use
+ * @param mutation the mutation to update
*/
- public LogEntry switchFile(String filePath) {
- return new LogEntry(filePath);
+ public void addToMutation(Mutation mutation) {
+
mutation.at().family(LogColumnFamily.NAME).qualifier(getColumnQualifier()).put(new
Value());
}
@Override
public String toString() {
- return filePath;
+ return logReference;
}
@Override
@@ -98,35 +101,29 @@ public class LogEntry {
return false;
}
LogEntry logEntry = (LogEntry) other;
- return this.filePath.equals(logEntry.filePath);
+ return this.logReference.equals(logEntry.logReference);
}
@Override
public int hashCode() {
- return Objects.hash(filePath);
+ return Objects.hash(logReference);
}
public static LogEntry fromMetaWalEntry(Entry<Key,Value> entry) {
- final Value value = entry.getValue();
-
- String filePath = value.toString();
-
- validateFilePath(filePath);
-
- return new LogEntry(filePath);
+ 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]);
}
public String getUniqueID() {
- String[] parts = filePath.split("/");
+ String[] parts = logReference.split("/");
return parts[parts.length - 1];
}
public Text getColumnQualifier() {
- return new Text("-/" + filePath);
- }
-
- public Value getValue() {
- return new Value(filePath);
+ return new Text("-/" + logReference);
}
}
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 d547b8596d..a4cb1b583c 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,7 +59,6 @@ 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;
@@ -104,11 +103,9 @@ public class TabletMetadataTest {
mutation.at().family(LastLocationColumnFamily.NAME).qualifier("s000").put("server2:8555");
LogEntry le1 = new LogEntry("localhost:8020/" + UUID.randomUUID());
-
mutation.at().family(LogColumnFamily.NAME).qualifier(le1.getColumnQualifier())
- .put(le1.getValue());
+ le1.addToMutation(mutation);
LogEntry le2 = new LogEntry("localhost:8020/" + UUID.randomUUID());
-
mutation.at().family(LogColumnFamily.NAME).qualifier(le2.getColumnQualifier())
- .put(le2.getValue());
+ le2.addToMutation(mutation);
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"));
@@ -140,8 +137,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.getValue(), le2.getValue()),
- tm.getLogs().stream().map(LogEntry::getValue).collect(toSet()));
+ assertEquals(Set.of(le1.getColumnQualifier(), le2.getColumnQualifier()),
+
tm.getLogs().stream().map(LogEntry::getColumnQualifier).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 302021121e..9fd5c3f1d2 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,20 +52,18 @@ public class LogEntryTest {
// test from constructor
LogEntry one = new LogEntry(validFilename);
assertEquals(validFilename, one.toString());
- assertEquals(validFilename, one.getFilePath());
+ assertEquals(validFilename, one.getLogReference());
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()),
one.getValue()));
+ new Key(new Text("1<"), new Text("log"), one.getColumnQualifier()),
new Value("unused")));
assertNotSame(one, two);
assertEquals(one.toString(), two.toString());
- assertEquals(one.getFilePath(), two.getFilePath());
+ assertEquals(one.getLogReference(), two.getLogReference());
assertEquals(one.getColumnQualifier(), two.getColumnQualifier());
assertEquals(one.getUniqueID(), two.getUniqueID());
- assertEquals(one.getValue(), two.getValue());
assertEquals(one, two);
}
@@ -76,10 +74,9 @@ public class LogEntryTest {
assertNotSame(one, two);
assertEquals(one.toString(), two.toString());
- assertEquals(one.getFilePath(), two.getFilePath());
+ assertEquals(one.getLogReference(), two.getLogReference());
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 98920aa2ea..29f470ebe6 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.getFilePath()), FileType.WAL,
replacements);
+ Path switchedPath = switchVolume(new Path(le.getLogReference()),
FileType.WAL, replacements);
String switchedString;
int numSwitched = 0;
if (switchedPath != null) {
switchedString = switchedPath.toString();
numSwitched++;
} else {
- switchedString = le.getFilePath();
+ switchedString = le.getLogReference();
}
if (numSwitched == 0) {
@@ -103,7 +103,7 @@ public class VolumeUtil {
return null;
}
- LogEntry newLogEntry = le.switchFile(switchedString);
+ LogEntry newLogEntry = new LogEntry(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.getFilePath(),
- switchedLogEntry.getFilePath());
+ log.debug("Replacing volume {} : {} -> {}", extent,
logEntry.getLogReference(),
+ switchedLogEntry.getLogReference());
} 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 c6bc4bef39..c628766b91 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.getFilePath()));
- log.debug("root tablet log {}", logEntry.getFilePath());
+ logs.add(Collections.singleton(logEntry.getLogReference()));
+ log.debug("root tablet log {}", logEntry.getLogReference());
});
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 73776db9b9..4f7f9c230c 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.");
- mutation.put(LogColumnFamily.NAME, logEntry.getColumnQualifier(),
logEntry.getValue());
+ logEntry.addToMutation(mutation);
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 2055d793d4..a3366facd7 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.getFilePath()));
+ volumes.add(getLogURI(logEntry.getLogReference()));
}
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 b6ac99845d..80db5f6906 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.getFilePath());
+ assertEquals("viewfs:/a/accumulo/wal/localhost+9997/" + walUUID,
fixedVolume.getLogReference());
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.getFilePath());
+ assertEquals("viewfs:/a/accumulo/wal/localhost+9997/" + walUUID,
fixedVolume.getLogReference());
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.getFilePath());
+ assertEquals("viewfs:/b/accumulo/wal/localhost+9997/" + walUUID,
fixedVolume.getLogReference());
}
}
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 5df7c75169..2a205f4cf4 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.getFilePath()));
+ Path finished = RecoveryPath.getRecoveryPath(new
Path(entry.getLogReference()));
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 1b6ba2f3cf..7c741729df 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.getFilePath(), logEntry.getColumnQualifier().toString()));
+ logEntry.getLogReference(),
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 983f9217d8..f6711ff223 100644
---
a/test/src/main/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
+++
b/test/src/main/java/org/apache/accumulo/test/MissingWalHeaderCompletesRecoveryIT.java
@@ -146,8 +146,7 @@ public class MissingWalHeaderCompletesRecoveryIT extends
ConfigurableMacBase {
Text row = TabletsSection.encodeRow(tableId, null);
Mutation m = new Mutation(row);
- m.put(TabletsSection.LogColumnFamily.NAME, logEntry.getColumnQualifier(),
- logEntry.getValue());
+ logEntry.addToMutation(m);
try (BatchWriter bw = client.createBatchWriter(MetadataTable.NAME)) {
bw.addMutation(m);
@@ -205,8 +204,7 @@ public class MissingWalHeaderCompletesRecoveryIT extends
ConfigurableMacBase {
Text row = TabletsSection.encodeRow(tableId, null);
Mutation m = new Mutation(row);
- m.put(TabletsSection.LogColumnFamily.NAME, logEntry.getColumnQualifier(),
- logEntry.getValue());
+ logEntry.addToMutation(m);
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 699a8f7eaf..40c2e07b5f 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,8 +204,7 @@ 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());
-
m.at().family(LogColumnFamily.NAME).qualifier(logEntry.getColumnQualifier())
- .put(logEntry.getValue());
+ logEntry.addToMutation(m);
update(accumuloClient, m);
// Verify state is still WAITING_FOR_OFFLINE
