Vrushali C commented on YARN-4178:

Thanks [~varun_saxena] for the patch. Overall, LGTM.  A couple of observations:

It would be good to have encodeAppId and decodeAppId in the same class instead 
of two different classes. 
To that effect, if you’d like, we can rename TimelineWriterUtils to 
TimelineStorageUtils so that both reader and writer can use functions from 
Also,let’s have the invert(long) and invert(int) functions in the same util 
class, instead of adding in a new util class.

While I do think we should store the “application_” prefix (if/when yarn starts 
allowing configurable prefixes so that we can see something like 
"spark_<cluster ts>_<seq_num>" or "tez_<cluster ts>_<seq_num>" on the cluster 
etc), I don’t want to hold up the jira for that since we could add it in later 
as we see fit.

> [storage implementation] app id as string in row keys can cause incorrect 
> ordering
> ----------------------------------------------------------------------------------
>                 Key: YARN-4178
>                 URL: https://issues.apache.org/jira/browse/YARN-4178
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>    Affects Versions: YARN-2928
>            Reporter: Sangjin Lee
>            Assignee: Varun Saxena
>         Attachments: YARN-4178-YARN-2928.01.patch, 
> YARN-4178-YARN-2928.02.patch
> Currently the app id is used in various places as part of row keys. However, 
> currently they are treated as strings. This will cause a problem with 
> ordering when the id portion of the app id rolls over to the next digit.
> For example, "app_1234567890_10000" will be considered *earlier* than 
> "app_1234567890_9999". We should correct this.

This message was sent by Atlassian JIRA

Reply via email to