You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/06/14 07:46:32 UTC

[GitHub] [ozone] adoroszlai opened a new pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

adoroszlai opened a new pull request #2330:
URL: https://github.com/apache/ozone/pull/2330


   ## What changes were proposed in this pull request?
   
   If Jaeger tracing is enabled key creation request fails due to:
   
   ```
   NoSuchMethodException: Method not found: allocateBlock
   	at org.apache.hadoop.hdds.tracing.TraceAllMethod.invoke(TraceAllMethod.java:65)
   	at com.sun.proxy.$Proxy36.allocateBlock(Unknown Source)
   	at org.apache.hadoop.ozone.om.request.key.OMKeyRequest.allocateBlock(OMKeyRequest.java:130)
   	at org.apache.hadoop.ozone.om.request.key.OMKeyCreateRequest.preExecute(OMKeyCreateRequest.java:151)
   	at org.apache.hadoop.ozone.protocolPB.OzoneManagerProtocolServerSideTranslatorPB.processRequest(OzoneManagerProtocolServerSideTranslatorPB.java:139)
   	at org.apache.hadoop.hdds.server.OzoneProtocolMessageDispatcher.processRequest(OzoneProtocolMessageDispatcher.java:87)
   	at org.apache.hadoop.ozone.protocolPB.OzoneManagerProtocolServerSideTranslatorPB.submitRequest(OzoneManagerProtocolServerSideTranslatorPB.java:122)
   ```
   
   https://issues.apache.org/jira/browse/HDDS-5335
   
   ## How was this patch tested?
   
   Tested key creation in `ozone` Docker Compose environment with Jaeger enabled:
   
   ```
   cd hadoop-ozone/dist/target/ozone-*/compose/ozone
   export COMPOSE_FILE=docker-compose.yaml:monitoring.yaml
   
   # console 1
   OZONE_REPLICATION_FACTOR=3 ./run.sh
   
   # console 2
   docker-compose exec -T scm ozone freon ockg -n1 -t1
   ```


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2330:
URL: https://github.com/apache/ozone/pull/2330#issuecomment-874468307


   > using `getMethods` instead of `getDeclaredMethods`
   
   @elek thanks for the suggestion, patch is updated, please take another look.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2330:
URL: https://github.com/apache/ozone/pull/2330#issuecomment-860422735


   > PS. I found the [jira](https://issues.apache.org/jira/browse/HDDS-5335) didn't show pull request available as usual, do you have any thoughts
   
   Thanks for pointing it out.  Opened [INFRA-21991](https://issues.apache.org/jira/browse/INFRA-21991).


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2330:
URL: https://github.com/apache/ozone/pull/2330#issuecomment-861590620


   Thanks @elek for the great suggestion.  I have pushed this implementation to a [separate branch](https://github.com/adoroszlai/hadoop-ozone/commits/HDDS-5335-alt).  Will add to this PR after #2339 is (hopefully) merged, to avoid the need for potentially triggering CI several times again.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] jojochuang commented on pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

Posted by GitBox <gi...@apache.org>.
jojochuang commented on pull request #2330:
URL: https://github.com/apache/ozone/pull/2330#issuecomment-860159636


   As a side node, the opentracing library has a mock test framework. I wonder if it's mature enough that we can use it to ensure traces are not broken or does function correctly.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek edited a comment on pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

Posted by GitBox <gi...@apache.org>.
elek edited a comment on pull request #2330:
URL: https://github.com/apache/ozone/pull/2330#issuecomment-861396282


   Ok, I checked the implementation and it looks like a bug in `TraceAllMethod` where we need using  `getMethods` instead of `getDeclaredMethods`
   
   I think `getDeclaredMethods` was used to avoid tracing the `Object` methods (`toString`,`equals`) but those can be excluded:
   
   The fixed version looks like:
   
   ```java
     public TraceAllMethod(T delegate, String name) {
       this.delegate = delegate;
       this.name = name;
       for (Method method : delegate.getClass().getMethods()) {
         if (!methods.containsKey(method.getName())) {
           methods.put(method.getName(), new HashMap<>());
         }
         if (!method.getDeclaringClass().equals(Object.class)) {
           methods.get(method.getName()).put(method.getParameterTypes(), method);
         }
       }
     }
   ``` 
   
   Tested with this unit test:
   
   ```java
   package org.apache.hadoop.hdds.tracing;
   
   import org.junit.Assert;
   import org.junit.Test;
   
   public class TestTraceAllMethod {
   
     @Test
     public void tracing() throws Throwable {
       final TraceAllMethod tracer = new TraceAllMethod(new ServiceImpl(), "test");
   
       Assert.assertEquals("Hello noname", tracer.invoke(null, ServiceImpl.class.getMethod("hello"), new Object[]{}));
       Assert.assertEquals("Hello asd", tracer.invoke(null, ServiceImpl.class.getMethod("hello", String.class), new Object[]{"asd"}));
     }
   
   
     public interface Service {
       default String hello() {
         return hello("noname");
       }
   
       String hello(String name);
     }
   
     public static class ServiceImpl implements Service {
   
       @Override
       public String hello(String name) {
         return "Hello " + name;
       }
     }
   }
   ```
   
   I think it would be greate to have this more resilient fix, but I am fine with both including it here or submitting it in a new PR.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek commented on pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

Posted by GitBox <gi...@apache.org>.
elek commented on pull request #2330:
URL: https://github.com/apache/ozone/pull/2330#issuecomment-861396282


   Ok, I checked the implementation and it looks like a bug in `TraceAllMethod` where we need using  `getMethods` instead of `getDeclaredMethods`
   
   I think `getDeclaredMethods` was used to avoid tracing the `Object` methods (`toString`,`equals`) but those can be excluded:
   
   The fixed version looks like:
   ```
     public TraceAllMethod(T delegate, String name) {
       this.delegate = delegate;
       this.name = name;
       for (Method method : delegate.getClass().getMethods()) {
         if (!methods.containsKey(method.getName())) {
           methods.put(method.getName(), new HashMap<>());
         }
         if (!method.getDeclaringClass().equals(Object.class)) {
           methods.get(method.getName()).put(method.getParameterTypes(), method);
         }
       }
     }
   ``` 
   
   Tested with this unit test:
   
   ```
   package org.apache.hadoop.hdds.tracing;
   
   import org.junit.Assert;
   import org.junit.Test;
   
   public class TestTraceAllMethod {
   
     @Test
     public void tracing() throws Throwable {
       final TraceAllMethod tracer = new TraceAllMethod(new ServiceImpl(), "test");
   
       Assert.assertEquals("Hello noname", tracer.invoke(null, ServiceImpl.class.getMethod("hello"), new Object[]{}));
       Assert.assertEquals("Hello asd", tracer.invoke(null, ServiceImpl.class.getMethod("hello", String.class), new Object[]{"asd"}));
     }
   
   
     public interface Service {
       default String hello() {
         return hello("noname");
       }
   
       String hello(String name);
     }
   
     public static class ServiceImpl implements Service {
   
       @Override
       public String hello(String name) {
         return "Hello " + name;
       }
     }
   }
   ```
   
   I think it would be greate to have this more resilient fix, but I am fine with both including it here or submitting it in a new PR.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai edited a comment on pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

Posted by GitBox <gi...@apache.org>.
adoroszlai edited a comment on pull request #2330:
URL: https://github.com/apache/ozone/pull/2330#issuecomment-919895687


   Thanks @avijayanhwx, @cxorm for the review, @elek for the suggestions incorporated in 27841671d394fb119549fbe53bc7ffbb331261fb.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai merged pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

Posted by GitBox <gi...@apache.org>.
adoroszlai merged pull request #2330:
URL: https://github.com/apache/ozone/pull/2330


   


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2330:
URL: https://github.com/apache/ozone/pull/2330#issuecomment-919895687


   Thanks @cxorm for the review, @elek for the suggestions incorporated in 27841671d394fb119549fbe53bc7ffbb331261fb.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2330:
URL: https://github.com/apache/ozone/pull/2330#issuecomment-900852489


   > > using `getMethods` instead of `getDeclaredMethods`
   > 
   > @elek thanks for the suggestion, patch is updated, please take another look.
   
   Friendly ping @elek.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2330:
URL: https://github.com/apache/ozone/pull/2330#issuecomment-900852489


   > > using `getMethods` instead of `getDeclaredMethods`
   > 
   > @elek thanks for the suggestion, patch is updated, please take another look.
   
   Friendly ping @elek.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek commented on pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

Posted by GitBox <gi...@apache.org>.
elek commented on pull request #2330:
URL: https://github.com/apache/ozone/pull/2330#issuecomment-861382417


   Thanks to fix this issue @adoroszlai 
   
   Can you please share more information, what was the problem? Looks to be related to the method overloading, but the TraceAllMethod supposed to handle that.
   
   I just try to understand the problem to avoid it next time...


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] adoroszlai commented on pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #2330:
URL: https://github.com/apache/ozone/pull/2330#issuecomment-874468307


   > using `getMethods` instead of `getDeclaredMethods`
   
   @elek thanks for the suggestion, patch is updated, please take another look.


-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org