ctubbsii commented on code in PR #4082:
URL: https://github.com/apache/accumulo/pull/4082#discussion_r1433309589
##########
core/src/main/java/org/apache/accumulo/core/tabletserver/log/LogEntry.java:
##########
@@ -27,103 +27,153 @@
import org.apache.accumulo.core.data.Value;
import
org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.LogColumnFamily;
import org.apache.hadoop.io.Text;
+import org.checkerframework.checker.nullness.qual.NonNull;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.net.HostAndPort;
-public class LogEntry {
+public final class LogEntry {
- private final String filePath;
+ private final String path;
+ private final HostAndPort tserver;
+ private final UUID uniqueId;
- public LogEntry(String filePath) {
- validateFilePath(filePath);
- this.filePath = filePath;
- }
-
- public String getFilePath() {
- return this.filePath;
+ private LogEntry(String path, HostAndPort tserver, UUID uniqueId) {
+ this.path = path;
+ this.tserver = tserver;
+ this.uniqueId = uniqueId;
}
/**
- * Validates the expected format of the file path. We expect the path to
contain a tserver
- * (host:port) followed by a UUID as the file name. For example,
- * localhost:1234/927ba659-d109-4bce-b0a5-bcbbcb9942a2 is a valid file path.
+ * Creates a new LogEntry object after validating the expected format of the
path. We expect the
+ * path to contain a tserver (host+port) followed by a UUID as the file name
as the last two
+ * components.<br>
+ * For example,
file:///some/dir/path/localhost+1234/927ba659-d109-4bce-b0a5-bcbbcb9942a2 is a
+ * valid path.
*
- * @param filePath path to validate
- * @throws IllegalArgumentException if the filePath is invalid
+ * @param path path to validate
+ * @return an object representation of this log entry
+ * @throws IllegalArgumentException if the path is invalid
*/
- private static void validateFilePath(String filePath) {
- String[] parts = filePath.split("/");
+ public static LogEntry fromPath(String path) {
+ String[] parts = path.split("/");
if (parts.length < 2) {
throw new IllegalArgumentException(
- "Invalid filePath format. The path should at least contain
tserver/UUID.");
+ "Invalid path format. The path should end with tserver/UUID.");
}
String tserverPart = parts[parts.length - 2];
String uuidPart = parts[parts.length - 1];
+ String badTServerMsg =
+ "Invalid tserver in path. Expected: host+port. Found '" + tserverPart
+ "'";
+ if (tserverPart.contains(":") || !tserverPart.contains("+")) {
+ throw new IllegalArgumentException(badTServerMsg);
+ }
+ HostAndPort tserver;
try {
- HostAndPort.fromString(tserverPart);
+ tserver = HostAndPort.fromString(tserverPart.replace("+", ":"));
Review Comment:
You can't use server.toString(), because it produces a string with a colon
in it, and that's [not valid for
HDFS](https://hadoop.apache.org/docs/r3.3.6/hadoop-project-dist/hadoop-common/filesystem/model.html).
When we write this to a file, we replace the colons with a plus sign, and
vice versa. That may not have been obvious in your tests because you used
java.io.file.Path, rather than Hadoop's Path type.
If you don't convert the plus sign to a colon, HostAndPort will still parse
it, but it will put everything in the host portion, and leave the port empty.
So:
```java
HostAndPort.fromString("localhost+8080"); // results in
{host=localhost+8080, port=}
HostAndPort.fromString("localhost:8080"); // results in {host=localhost,
port=8080}
```
It's hard to tell, because HostAndPort.toString() looks the same in both
cases.
When we write the file to disk, we actually convert it to use the plus sign:
https://github.com/apache/accumulo/blob/f9897862dd4e6ff4892239ff5ebeb8ed6e34bc68/server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java#L384
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
