DomGarguilo commented on code in PR #4192:
URL: https://github.com/apache/accumulo/pull/4192#discussion_r1466687596
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/log/DfsLogger.java:
##########
@@ -291,16 +291,36 @@ public int hashCode() {
private final ServerContext context;
private FSDataOutputStream logFile;
private DataOutputStream encryptingLogFile = null;
- private LogEntry logEntry;
+ private final LogEntry logEntry;
private Thread syncThread;
private AtomicLong syncCounter;
private AtomicLong flushCounter;
private final long slowFlushMillis;
private long writes = 0;
- public DfsLogger(ServerContext context, AtomicLong syncCounter, AtomicLong
flushCounter) {
- this(context, null);
+ public static DfsLogger fromCounters(ServerContext context, AtomicLong
syncCounter,
+ AtomicLong flushCounter, String address) {
+
+ String filename = UUID.randomUUID().toString();
+ String logger = Joiner.on("+").join(address.split(":"));
+ VolumeManager fs = context.getVolumeManager();
+
+ var chooserEnv =
+ new
VolumeChooserEnvironmentImpl(VolumeChooserEnvironment.Scope.LOGGER, context);
+ String logPath = fs.choose(chooserEnv, context.getBaseUris()) +
Path.SEPARATOR
+ + Constants.WAL_DIR + Path.SEPARATOR + logger + Path.SEPARATOR +
filename;
+
+ return new DfsLogger(context, syncCounter, flushCounter,
LogEntry.fromPath(logPath));
+ }
+
+ public static DfsLogger fromExistingLogEntry(ServerContext context, LogEntry
logEntry) {
+ return new DfsLogger(context, logEntry);
+ }
+
+ private DfsLogger(ServerContext context, AtomicLong syncCounter, AtomicLong
flushCounter,
+ LogEntry logEntry) {
Review Comment:
This is pretty similar to another approach that I thought of that makes it
so we can call open within the `fromCounters` method.
What we might be able to do is make a single private constructor that sets
all of the fields that both of these static methods use. The
`fromExistingLogEntry` is more straight forward where we can just call the
private constructor but the `fromCounters` we could do the following:
After creating the `String logPath`, create the `LogEntry` and use all of
the variables to create a DfsLogger object to then call `open` on. Then return
that DfsLogger. Like this:
```java
String logPath = fs.choose(chooserEnv, context.getBaseUris()) +
Path.SEPARATOR
+ Constants.WAL_DIR + Path.SEPARATOR + loggerAddress +
Path.SEPARATOR + filename;
LogEntry logEntry = LogEntry.fromPath(logPath);
DfsLogger logger = new DfsLogger(context, logEntry, syncCounter,
flushCounter);
logger.open(fs, logPath, filename);
return logger;
```
That way we can make open private too and LogEntry can be final.
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/log/TabletServerLogger.java:
##########
@@ -266,7 +266,8 @@ private synchronized void startLogMaker() {
DfsLogger alog = null;
try {
- alog = new DfsLogger(tserver.getContext(), syncCounter,
flushCounter);
+ alog = DfsLogger.fromCounters(tserver.getContext(), syncCounter,
flushCounter,
+ tserver.getClientAddressString());
alog.open(tserver.getClientAddressString());
Review Comment:
This is the spot referenced in the ticket and I think can be addressed by my
other comments.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
