[jira] [Comment Edited] (YARN-6256) Add FROM_ID info key for timeline entities in reader response.
[ https://issues.apache.org/jira/browse/YARN-6256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15894962#comment-15894962 ] Varun Saxena edited comment on YARN-6256 at 3/3/17 8:12 PM: Thanks [~rohithsharma] for the patch. It looks pretty close. A couple of nits. # When we are encoding and decoding as strings, app ids', flow run id, etc. cannot have separator char i.e."!". So the static block in TestRowKeysAsString is not required. It was required in TestRowKeys because when we convert long / int to bytes it may have same byte sequence as separator char so that had to be tested. In TestRowKeysAsString we can use any fixed flow run ID and App ID. We do not need to use Separator#QUALIFIERS in the test class as well. # There are changes in TimelineEntityFilters javadoc which is not really necessary. It is there due to some line formatting. This unnecessarily increases size of the patch. If possible exclude these changes. # In testEntityRowKey better to use separator and escape char constants. Other than this, it looks fine. Checkstyle need not be fixed. was (Author: varun_saxena): Thanks [~rohithsharma] for the patch. It looks pretty close. A couple of nits. # When we are encoding and decoding as strings, app ids', flow run id, etc. cannot have separator char i.e."!". So the static block in TestRowKeysAsString is not required. It was required in TestRowKeys because when we convert long / int to bytes it may have same byte sequence as separator char so that had to be tested. In TestRowKeysAsString we can use any fixed flow run ID and App ID. We do not need to use Separator#QUALIFIERS in the test class as well. # There are changes in TimelineEntityFilters javadoc which is not really necessary. It is there due to some line formatting. This unnecessarily increases size of the patch. If possible exclude these changes. Other than this, it looks fine. Checkstyle need not be fixed. > Add FROM_ID info key for timeline entities in reader response. > --- > > Key: YARN-6256 > URL: https://issues.apache.org/jira/browse/YARN-6256 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Rohith Sharma K S >Assignee: Rohith Sharma K S > Labels: yarn-5355-merge-blocker > Attachments: YARN-6256-YARN-5355.0001.patch, > YARN-6256-YARN-5355.0002.patch > > > It is continuation with YARN-6027 to add FROM_ID key in all other timeline > entity responses which includes > # Flow run entity response. > # Application entity response > # Generic timeline entity response - Here we need to retrospect on idprefix > filter which is now separately provided. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Comment Edited] (YARN-6256) Add FROM_ID info key for timeline entities in reader response.
[ https://issues.apache.org/jira/browse/YARN-6256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15894621#comment-15894621 ] Sangjin Lee edited comment on YARN-6256 at 3/3/17 4:01 PM: --- Point taken. I know some entity ids and flow name etc. can get quite long, so it's pretty easy for such a from-id field to be in the hundreds of bytes. But again, if we return 100 entities, that would add only tens of kB. If we are going to keep it for all entities, we might as well keep it for single-entity queries too for consistency. was (Author: sjlee0): Point taken. I know some entity ids and flow name etc. can get quite long, so it's pretty easy for such a from-id field to be in the hundreds of bytes. But again, if we return 100 entities, that would add only tens of kB. > Add FROM_ID info key for timeline entities in reader response. > --- > > Key: YARN-6256 > URL: https://issues.apache.org/jira/browse/YARN-6256 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Rohith Sharma K S >Assignee: Rohith Sharma K S > Labels: yarn-5355-merge-blocker > Attachments: YARN-6256-YARN-5355.0001.patch > > > It is continuation with YARN-6027 to add FROM_ID key in all other timeline > entity responses which includes > # Flow run entity response. > # Application entity response > # Generic timeline entity response - Here we need to retrospect on idprefix > filter which is now separately provided. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Comment Edited] (YARN-6256) Add FROM_ID info key for timeline entities in reader response.
[ https://issues.apache.org/jira/browse/YARN-6256?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15892935#comment-15892935 ] Sangjin Lee edited comment on YARN-6256 at 3/2/17 8:35 PM: --- Thanks for the patch [~rohithsharma]! I took a quick look at it, and I still need to do a more thorough review, but my observations so far are similar to Varun's above. I do have another question (which also applies to YARN-6027). It seems that we're embedding the FROMID value for every single entity we return. Would it be possible to do this only for the "last" entity that gets returned if we're returning multiple entities? This might be a secondary optimization, but for all other entities but the last, the FROMID value would be ignored. In that sense, it would simply add to the payload size without providing benefits. Thoughts? was (Author: sjlee0): Thanks for the patch [~rohithsharma]! I took a quick look at it, and I still need to do a more thorough review, but my observations so far are similar to Varun's above. I do have another questions (which also applies to YARN-6027). It seems that we're embedding the FROMID value for every single entity we return. Would it be possible to do this only for the "last" entity that gets returned if we're returning multiple entities? This might be a secondary optimization, but for all other entities but the last, the FROMID value would be ignored. In that sense, it would simply add to the payload size without providing benefits. Thoughts? > Add FROM_ID info key for timeline entities in reader response. > --- > > Key: YARN-6256 > URL: https://issues.apache.org/jira/browse/YARN-6256 > Project: Hadoop YARN > Issue Type: Sub-task > Components: timelineserver >Reporter: Rohith Sharma K S >Assignee: Rohith Sharma K S > Labels: yarn-5355-merge-blocker > Attachments: YARN-6256-YARN-5355.0001.patch > > > It is continuation with YARN-6027 to add FROM_ID key in all other timeline > entity responses which includes > # Flow run entity response. > # Application entity response > # Generic timeline entity response - Here we need to retrospect on idprefix > filter which is now separately provided. -- This message was sent by Atlassian JIRA (v6.3.15#6346) - To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
