Thanks for the reviews Allen Wittenauer and Jian He.
Why are we doing this manipulation here and not in the Java code?
I get different values when I run the command in the yarn script vs spawn it via Java. From Java, I get lower values - 80x24, whereas the yarn script gives me 204x44.
backticks are antiquated in modern bash. Use $() construction
What happens if tput gives you zero or an error because you are on a non-addressable terminal? (You can generally simulate this by unset TERM or equivalent env var)
Thank you for pointing this out. I hadn't considered it. I've added additional checks in the script. If the values can't be determined either by the script or by the Java code, it sets it to 80x24.
“Unable to fetach cluster metrics” - typo
exceeding 80 Column limit,
the -rows, -cols options seems not having effect on my screen when I tried it, could you double check ?
I found an issue with cols option which I've fixed. Can you please try it again?
the ‘yarn top’ output is repeatedly showing up on terminal every $delay seconds. it’ll be better to only show that only once.
I didn't understand this - do you mean that it shouldn't auto-refresh?
Does the patch only show root queue info ? should we show all queues info ?
Queues can be specified as a comma seperated string using the "-queues" option. By default, it shows information for the root queue.
“F + Enter : Select sort field” ; may be use ’S’ for sorting ?
“Memory seconds(in GBseconds” - missing “)”
It seems a bit odd to have this method in a public API record. Do you know why hashcode is not correct without this method ? Or we can just type cast it to GetApplicationsRequestPBImpl and use the method from there.
// need this otherwise the hashcode doesn't get generated correctly
for the caching in ClientRMService. Do you think we can do the cache on client side ? that’ll save RPCs, especially if we have many top commands running on client side.
Fixed. Moved the cache to YarnClientImpl where the hashcode doesn't show up. As to why it wasn't correct - I suspect it might be to do with lazy initialization but I'm not sure.