Sangjin Lee commented on YARN-4025:

Thanks for your review [~djp]!

Do we handle columnPrefixBytes to be null here as Javadoc comments? I saw we 
handle this case explicitly in readResults() but I didn't see it here. Let me 
know if I miss something.

That is a good catch. Let me look into that. If we retain the same behavior for 
a null qualifier (and probably we should), then the return type of this method 
would need to go back to {{Map<Object, Object>}}. I'll also think about the 
method names. Cc'ing [~vrushalic] for her opinion also.

Checking with javadoc in Separator and TimelineWriterUtils - "a negative value 
indicates no limit on number of segments.", so can we define a constant value 
like NO_LIMIT to replace -1 here?

Will do.

I think we should do the same thing to some javadoc examples in 

The {{EntityTable.java}} file is already fixed in the v.3 patch.

Forget to mention that, YARN-3049 should rename TestHBaseTimelineWriterImpl to 
something include Reader. Would you like to do it here? Thanks!

Good idea. The thought definitely occurred to me.

I'll update the patch pretty soon.

> Deal with byte representations of Longs in writer code
> ------------------------------------------------------
>                 Key: YARN-4025
>                 URL: https://issues.apache.org/jira/browse/YARN-4025
>             Project: Hadoop YARN
>          Issue Type: Sub-task
>          Components: timelineserver
>            Reporter: Vrushali C
>            Assignee: Sangjin Lee
>         Attachments: YARN-4025-YARN-2928.001.patch, 
> YARN-4025-YARN-2928.002.patch, YARN-4025-YARN-2928.003.patch
> Timestamps are being stored as Longs in hbase by the HBaseTimelineWriterImpl 
> code. There seem to be some places in the code where there are conversions 
> between Long to byte[] to String for easier argument passing between function 
> calls. Then these values end up being converted back to byte[] while storing 
> in hbase. 
> It would be better to pass around byte[] or the Longs themselves  as 
> applicable. 
> This may result in some api changes (store function) as well in adding a few 
> more function calls like getColumnQualifier which accepts a pre-encoded byte 
> array. It will be in addition to the existing api which accepts a String and 
> the ColumnHelper to return a byte[] column name instead of a String one. 
> Filing jira to track these changes.

This message was sent by Atlassian JIRA

Reply via email to