YARN-11219. [Federation] Add getAppActivities, getAppStatistics REST APIs for Router. #4757
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
💔 -1 overall
This message was automatically generated. |
goiri
reviewed
Aug 19, 2022
| public void add(StatisticsItemInfo statItem) { | ||
| this.statItem.add(statItem); | ||
| } | ||
|
|
||
| public ArrayList<StatisticsItemInfo> getStatItems() { | ||
| return statItem; | ||
| } | ||
|
|
There was a problem hiding this comment.
Thanks for your help reviewing the code, I will modify the code.
| return interceptor.getAppActivities(hsrCopy, appId, time, requestPriorities, | ||
| allocationRequestIds, groupBy, limit, actions, summarize); | ||
| } catch (IllegalArgumentException e) { | ||
| RouterServerUtil.logAndThrowRunTimeException(e, |
| String statisticsItemKey = statisticsItemInfo.getType() + "_" + statisticsItemInfo.getState().toString(); | ||
| StatisticsItemInfo statisticsItemValue = | ||
| statisticsItemMap.getOrDefault(statisticsItemKey, null); | ||
| if (statisticsItemValue != null) { |
There was a problem hiding this comment.
Cleaner to do contains or similar.
|
|
||
| for (HashMap.Entry<ApplicationId, ApplicationReport> item : applicationMap.entrySet()) { | ||
|
|
||
| ApplicationReport applicationReport = item.getValue(); |
There was a problem hiding this comment.
If we don't do getKey, we can iterate .values()
|
|
||
| ArrayList<StatisticsItemInfo> itemInfos = new ArrayList<>(itemInfoMap.values()); | ||
|
|
||
| return new ApplicationStatisticsInfo(itemInfos); |
There was a problem hiding this comment.
You could do:
return new ApplicationStatisticsInfo(itemInfoMap.values());
There was a problem hiding this comment.
Thank you for your suggestion, I will modify the code.
| throw new NotFoundException("app with id: " + appId + " not found"); | ||
| } | ||
|
|
||
| SchedulerNode schedulerNode = |
|
|
||
| Assert.assertEquals(1, mergeInfo.getStatItems().size()); | ||
| Assert.assertEquals(item1.getCount() + item2.getCount(), | ||
| mergeInfo.getStatItems().get(0).getCount()); |
There was a problem hiding this comment.
extract get(0) and check it first.
There was a problem hiding this comment.
I will modify the code.
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
@goiri Please help to review the code again, thank you very much! |
goiri
reviewed
Aug 20, 2022
| public void add(StatisticsItemInfo statItem) { | ||
| this.statItem.add(statItem); | ||
| } | ||
|
|
||
| public ArrayList<StatisticsItemInfo> getStatItems() { | ||
| return statItem; | ||
| } | ||
|
|
||
| } | ||
| } |
| import org.apache.hadoop.yarn.server.resourcemanager.scheduler.ActiveUsersManager; | ||
| import org.apache.hadoop.yarn.server.resourcemanager.scheduler.ResourceScheduler; | ||
| import org.apache.hadoop.yarn.server.resourcemanager.scheduler.SchedulerNode; | ||
| import org.apache.hadoop.yarn.server.resourcemanager.scheduler.activities.*; |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
@goiri Please help to review the code again, thank you very much! |
goiri
reviewed
Aug 22, 2022
| statisticsItemInfo.getType() + "_" + statisticsItemInfo.getState().toString(); | ||
|
|
||
| StatisticsItemInfo statisticsItemValue; | ||
| if(statisticsItemMap.containsKey(statisticsItemKey)) { |
There was a problem hiding this comment.
Thanks for your help reviewing the code, I will fix it.
| interceptor.getAppStatistics(null, stateQueries, typeQueries); | ||
|
|
||
| Assert.assertNotNull(response2); | ||
| Assert.assertTrue(!response2.getStatItems().isEmpty()); |
| ApplicationStatisticsInfo infoA = new ApplicationStatisticsInfo(); | ||
| ApplicationStatisticsInfo infoB = new ApplicationStatisticsInfo(); | ||
|
|
||
| StatisticsItemInfo item1 = |
| @Test | ||
| public void testMergeDiffApplicationStatisticsInfo() { | ||
| ApplicationStatisticsInfo infoA = new ApplicationStatisticsInfo(); | ||
| StatisticsItemInfo item1 = |
| RouterWebServiceUtil.mergeApplicationStatisticsInfo(lists); | ||
|
|
||
| Assert.assertEquals(3, mergeInfo.getStatItems().size()); | ||
| ArrayList<StatisticsItemInfo> mergeInfoStatItems = mergeInfo.getStatItems(); |
There was a problem hiding this comment.
List<StatisticsItemInfo> mergeInfoStatItems
| } | ||
| } | ||
|
|
||
| Assert.assertEquals(item1.getType(), item1Result.getType()); |
There was a problem hiding this comment.
Can we do:
assertEquals(YarnApplicationState.ACCEPTED, item1Result.getState());
There was a problem hiding this comment.
Thanks for your suggestion, I will modify the code.
|
🎊 +1 overall
This message was automatically generated. |
|
@goiri Please help to review the code again, Thank you very much! |
goiri
approved these changes
Aug 24, 2022
|
@goiri Can you help to merge this pr into trunk branch? I will follow up with YARN-11226. Thank you very much! |
|
@goiri Thank you very much for helping to review the code! |
HarshitGupta11
pushed a commit
to HarshitGupta11/hadoop
that referenced
this pull request
Nov 28, 2022
…APIs for Router. (apache#4757)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JIRA:YARN-11219. [Federation] Add getAppActivities, getAppStatistics REST APIs for Router.