[ 
https://issues.apache.org/jira/browse/HDFS-14284?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16960790#comment-16960790
 ] 

Ayush Saxena commented on HDFS-14284:
-------------------------------------

Thanx [~hemanthboyina] for the patch. Couple of comments :

{code:java}
-        throw new NoNamenodesAvailableException(nsId, ioe);
+        throw new NoNamenodesAvailableException(
+            nsId + " from router " + router.getRouterId(), ioe);
{code}

No need to append the text here in the nsId variable, Doesn't make sense to 
have message for a variable which intends to store NsId, Add a param for Router 
ID, and do the message appending part and all inside the Exception method, To 
make the actual code flow look clean.


{code:java}
-      throw new IOException("No namenodes to invoke " + method.getName() +
-          " with params " + Arrays.deepToString(params) + " from "
-          + router.getRouterId());
+      throw new RouterIOException("No namenodes to invoke " + method.getName()
+          + " with params " + Arrays.deepToString(params),
+          router.getRouterId());
{code}

If I see earlier the text was from ROUTERID not from router ROUTERID

{code:java}
.append(" from router ")
{code}
So, better we keep the text same, don't add router here, Somebody parsing the 
string would fail, if we tweak the text.

For the test :

* Derrive the RouterIOException from the RemoteException, And check the 
{{getMessage}} and {{getRouterID}} are giving correct stuff.
* No need to remove the NoNamenodeException Test, We are changing that too, 
Better keep that too, If the both exceptions are using some code flow, put them 
in same test and name the test a little genreic, otherwise sepaerate them, But 
try to reuse the code if possible, by refactoring into a method, if you can't 
keep in one test.
* 



> RBF: Log Router identifier when reporting exceptions
> ----------------------------------------------------
>
>                 Key: HDFS-14284
>                 URL: https://issues.apache.org/jira/browse/HDFS-14284
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>            Reporter: Íñigo Goiri
>            Assignee: hemanthboyina
>            Priority: Major
>         Attachments: HDFS-14284.001.patch, HDFS-14284.002.patch, 
> HDFS-14284.003.patch, HDFS-14284.004.patch, HDFS-14284.005.patch, 
> HDFS-14284.006.patch, HDFS-14284.007.patch
>
>
> The typical setup is to use multiple Routers through 
> ConfiguredFailoverProxyProvider.
> In a regular HA Namenode setup, it is easy to know which NN was used.
> However, in RBF, any Router can be the one reporting the exception and it is 
> hard to know which was the one.
> We should have a way to identify which Router/Namenode was the one triggering 
> the exception.
> This would also apply with Observer Namenodes.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to