K0K0V0K commented on code in PR #8556:
URL: https://github.com/apache/hadoop/pull/8556#discussion_r3644456248
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/filecontroller/ifile/LogAggregationIndexedFileController.java:
##########
@@ -121,22 +121,41 @@ public class LogAggregationIndexedFileController
private int fsNumRetries = 3;
private long fsRetryInterval = 1000L;
private static final int VERSION = 1;
- private IndexedLogsMeta indexedLogsMeta = null;
- private IndexedPerAggregationLogMeta logsMetaInThisCycle;
- private long logAggregationTimeInThisCycle;
- private FSDataOutputStream fsDataOStream;
private Algorithm compressAlgo;
- private CachedIndexedLogsMeta cachedIndexedLogsMeta = null;
- private boolean logAggregationSuccessfullyInThisCyCle = false;
- private long currentOffSet = 0;
- private Path remoteLogCheckSumFile;
- private FileContext fc;
- private UserGroupInformation ugi;
- private byte[] uuid = null;
private final int UUID_LENGTH = 32;
private long logRollOverMaxFileSize;
private Clock sysClock;
+ /**
+ * All mutable state that belongs to a single write session
+ * (one {@link #initializeWriter} / {@link #write} / {@link #postWrite} /
+ * {@link #closeWriter} lifecycle). Bundling it here means the read path
+ * cannot accidentally touch write-path state, regardless of whether the
+ * controller instance is shared across applications.
+ */
+ private static final class WriteSession {
+ /** UUID derived from the application being written. */
+ private final byte[] uuid;
+ /** Accumulated log metadata for the current aggregated file. */
+ private IndexedLogsMeta indexedLogsMeta;
+ /** Log metadata accumulated within this single write cycle. */
+ private IndexedPerAggregationLogMeta logsMetaInThisCycle;
+ private long logAggregationTimeInThisCycle;
+ private boolean logAggregationSuccessfullyInThisCyCle = false;
+ private long currentOffSet = 0;
+ private Path remoteLogCheckSumFile;
+ private FileContext fc;
+ private UserGroupInformation ugi;
+ private FSDataOutputStream fsDataOStream;
+
+ WriteSession(byte[] uuid) {
+ this.uuid = uuid;
+ }
+ }
+
+ /** Non-null while a write session is in progress; null otherwise. */
+ private WriteSession writeSession = null;
Review Comment:
Thanks @ferdelyi for the quick fix!
Now i am wondering this state does not have some concurrency problem.
The `#initializeWriter` says:
```
// Allocate a fresh WriteSession for every initializeWriter call so that
// all write-path state is completely isolated from other applications.
// This is safe whether the controller is used by a single NM aggregation
// thread or reused across requests by a singleton web service.
```
But after that the newly created object will be set like
```
this.writeSession = session;
```
So if we call
```
controller.initializeWriter(app1Context);
controller.initializeWriter(app2Context);
```
than writer of app1 will be overwritten with app2 context, and when
controller try to write app1 in fact will write to app2 location?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
