Thanks Xuan Gong for updating the patch. I just go through the latest patch, and below is my review comments:
Can we add Javadoc for TimelineEntityGroupId and explain what TimelineEntityGroupId is used for?
Also in TimelineEntityGroupId.equals(), can we omit unnecessary ";"?
+ "=" + flushIntervalSecs + ", " +
+ "=" + cleanIntervalSecs + ", " +
+ "=" + ttl + ", " +
+ "=" + isAppendSupported);
Shall we log other related configurations, like: activePath, retryPolicy, summaryEntityTypes, etc.?
The name of entitiesToSummary, entitiesToEntity and entitiesToDB sounds a little confusing. How about update them to entitiesToSummaryCache, entitiesToEntityCache and entitiesToDBStore? Also, we shouldn't log in INFO level for each entity get written in cache. Debug level should be good.
summanyLogFDs = new HashMap<ApplicationAttemptId, EntityLogFD>();
entityLogFDs = new HashMap<ApplicationAttemptId,
There is a race condition here: if LogFDsCache is doing flush with iterating the map and some writing entity/summary come to insert the HashMap at the same time, then it will throw ConcurrentModificationException. We should use ConcurrentHashMap instead.
Also, for CleanInActiveFDsTask, We should use WARN level for LOG instead of DEBUG in case exception get throw.
The conf seems not get used. If so, we can remove it from class and constructor?
Other looks good to me.