Junping Du commented on YARN-2288:

Thanks [~zjshen] for review and comments! 
bq. Should we do the similar thing for MemoryTimelineStore? Though it is used 
to for testing and playing the feature, we never prevent users from choosing 
it, and it is subject to changes in the future as well.
Do we have plan to persistent MemoryTimelineStore? If not, I don't think it is 
necessary to have a version here as it is used to aware old format data get 
loaded into new code during rollup. If objects in store will get lost after TS 
restart, we don't need it. What do you think?

bq. Should we move the constants to TimelineStore, such that it can be 
inherited by different store implementation. If CURRENT_VERSION_INFO may differ 
for different stores, it can be kept here.
I don't understand what's different TimelineStore implementations could need 
this in future. If so, that could be possible to be different number. From my 
above comments, we only need this for LeveldbTimelineStore today. Let's keep it 
simple? We can refactor it if need it in future.

bq. The one-line wrapper method seems to be unnecessary.
Nice catch. Will fix it and other incorrect comments (sorry for that copy-paste 

bq.  Log an fatal message here? 
Nice suggestion. Will add it there.

> Data persistent in timelinestore should be versioned
> ----------------------------------------------------
>                 Key: YARN-2288
>                 URL: https://issues.apache.org/jira/browse/YARN-2288
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>    Affects Versions: 2.4.1
>            Reporter: Junping Du
>            Assignee: Junping Du
>         Attachments: YARN-2288.patch
> We have LevelDB-backed TimelineStore, it should have schema version for 
> changes in schema in future.

This message was sent by Atlassian JIRA

Reply via email to