You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "goiri (via GitHub)" <gi...@apache.org> on 2023/05/29 17:36:05 UTC
[GitHub] [hadoop] goiri commented on a diff in pull request #5696: HDFS-16946. Fix getTopTokenRealOwners to return String
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-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org