goiri commented on code in PR #5696:
URL: https://github.com/apache/hadoop/pull/5696#discussion_r1209480436
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/RBFMetrics.java:
##########
@@ -50,6 +50,8 @@
import javax.management.ObjectName;
import javax.management.StandardMBean;
+import com.fasterxml.jackson.core.JsonProcessingException;
Review Comment:
What's our take on using fasterxml in HDFS?
Is that the common approach?
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/RBFMetrics.java:
##########
@@ -712,13 +715,22 @@ public long getCurrentTokensCount() {
@Override
public String getTopTokenRealOwners() {
- RouterSecurityManager mgr =
+ String topTokenRealOwnersString = "";
+ RouterSecurityManager routerSecurityManager =
this.router.getRpcServer().getRouterSecurityManager();
- if (mgr != null && mgr.getSecretManager() != null) {
- return JSON.toString(mgr.getSecretManager()
- .getTopTokenRealOwners(this.topTokenRealOwners));
+ if (routerSecurityManager != null &&
routerSecurityManager.getSecretManager() != null) {
+ try {
+ List<Metrics2Util.NameValuePair> topOwners =
routerSecurityManager.getSecretManager()
+ .getTopTokenRealOwners(this.topTokenRealOwners);
+ if (topOwners.size() > 0) {
Review Comment:
```suggestion
if (!topOwners.isEmpty()) {
```
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/RBFMetrics.java:
##########
@@ -712,13 +715,22 @@ public long getCurrentTokensCount() {
@Override
public String getTopTokenRealOwners() {
- RouterSecurityManager mgr =
+ String topTokenRealOwnersString = "";
+ RouterSecurityManager routerSecurityManager =
this.router.getRpcServer().getRouterSecurityManager();
- if (mgr != null && mgr.getSecretManager() != null) {
- return JSON.toString(mgr.getSecretManager()
- .getTopTokenRealOwners(this.topTokenRealOwners));
+ if (routerSecurityManager != null &&
routerSecurityManager.getSecretManager() != null) {
Review Comment:
Let's reverse the if.
```
if (routerSecurityManager == null ||
routerSecurityManager.getSecretManager() == null) {
return topTokenRealOwnersString;
}
```
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/RBFMetrics.java:
##########
@@ -712,13 +715,22 @@ public long getCurrentTokensCount() {
@Override
public String getTopTokenRealOwners() {
Review Comment:
Do we have a unit test for this?
##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/metrics/RBFMetrics.java:
##########
@@ -712,13 +715,22 @@ public long getCurrentTokensCount() {
@Override
public String getTopTokenRealOwners() {
- RouterSecurityManager mgr =
+ String topTokenRealOwnersString = "";
+ RouterSecurityManager routerSecurityManager =
this.router.getRpcServer().getRouterSecurityManager();
- if (mgr != null && mgr.getSecretManager() != null) {
- return JSON.toString(mgr.getSecretManager()
- .getTopTokenRealOwners(this.topTokenRealOwners));
+ if (routerSecurityManager != null &&
routerSecurityManager.getSecretManager() != null) {
+ try {
+ List<Metrics2Util.NameValuePair> topOwners =
routerSecurityManager.getSecretManager()
+ .getTopTokenRealOwners(this.topTokenRealOwners);
+ if (topOwners.size() > 0) {
+ topTokenRealOwnersString = new
ObjectMapper().writeValueAsString(topOwners);
+ }
+ }
+ catch (JsonProcessingException jsonProcessingException) {
+ LOG.error("Unable to parse the top token real owners to string" +
jsonProcessingException.getMessage());
Review Comment:
Use {}
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
