ferdelyi commented on PR #8556:
URL: https://github.com/apache/hadoop/pull/8556#issuecomment-5068296108

   > Hi @ferdelyi !
   > 
   > Thanks for fixing this bug!
   > 
   > > After [HADOOP-15984](https://issues.apache.org/jira/browse/HADOOP-15984) 
made HsWebServices a singleton, the same controller instance handles requests 
for multiple applications. The instance field this.uuid would hold the UUID of 
the first application ever read. Every subsequent app's log files would fail 
validation with "UUID mismatch" and return no results — until JHS restarted.
   > 
   > Please correct me if i am wrong but in that case every other place where 
the `this.uuid` is in used can be a potential bug right? For example line #212. 
Can you please check can we (/should we) refactor this class to remove 
this.uuid? and maybe other class varaibles also? Or can we restore the original 
behaviour when the class was not singleton?
   
   Thank you for your review @K0K0V0K , good call!
   
   You are correct: this.uuid at lines 212, 271-273, 323, and 443 are all 
write-path uses that continued to work correctly after the initial fix, but the 
instance field remained a latent source of confusion — it was only safe because 
the read path happened not to depend on it after the patch, not because the 
design made that dependency impossible to introduce.
   
   To address this: all mutable write-session state (uuid, indexedLogsMeta, 
logsMetaInThisCycle, logAggregationTimeInThisCycle, 
logAggregationSuccessfullyInThisCyCle, currentOffSet, remoteLogCheckSumFile, 
fc, ugi, fsDataOStream) has been moved into a new private static inner class 
WriteSession. initializeWriter now allocates a fresh WriteSession on every 
invocation, so each write lifecycle is fully self-contained and independent of 
any previous one.
   
   Summary:
   - There are no longer any write-path instance fields on the controller at 
all, so it is impossible for the read path to accidentally access write-path 
state.
   - The currentWriteAppId sentinel guard introduced in the initial fix is no 
longer needed and has been removed — since a fresh WriteSession is created 
unconditionally on every initializeWriter call, there is no accumulated state 
to guard against.
   - The controller is now safe to reuse across applications both in the NM 
aggregation thread and in the singleton JHS web service, by construction rather 
than by convention.


-- 
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]

Reply via email to