You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/05/12 07:22:45 UTC

[GitHub] [pulsar] devinbost opened a new pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

devinbost opened a new pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548


   During testing/debugging, I was getting NPEs when writing out the value of OpAddEntry. This PR prevents NPEs from occurring when calling toString() when the OpAddEntry has null for its managed ledger instance or ledger instance. 


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



[GitHub] [pulsar] eolivelli commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-839641390


   you can split the implementation of toString in several instructions, this way you will have a more accurate stacktrace for the error 


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



[GitHub] [pulsar] devinbost commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
devinbost commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-839584440


   I could wrap `ml.getName()` in a null check, like:
   `"mlName=" + ml != null ? (ml.getName() != null ? ml.getName() : "null") : "null" +`


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



[GitHub] [pulsar] devinbost commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
devinbost commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-839646529


   @eolivelli Thanks for the advice. As I'm thinking about it, you're right that I probably don't need to log the value of the recycled object before setting the new values. When I added that line, I was wanting to see the before/after since I wanted a complete picture, but it's probably not adding much useful information in that line. 


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



[GitHub] [pulsar] lhotari commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-839754494


   > I tested this, and I'm still getting an NPE on this line:
   > `"mlName=" + ml != null ? ml.getName() : "null"`
   > Looks like `ml.getName()` is just returning `name`...
   > Any ideas?
   
   @devinbost  Code like `"mlName=" + ml != null ? ml.getName() : "null"` can fail when there isn't proper synchronization in place and the field has been modified by another thread.
   One workaround would be to store the field values into local variables and then do the null checks.
   Something like this:
   ```
       @Override
       public String toString() {
           ManagedLedgerImpl ml = this.ml;
           LedgerHandle ledger = this.ledger;
           return "OpAddEntry{"
                   + "mlName" + ml != null ? ml.getName() : "null"
                   + ", ledgerId=" + ledger != null ? String.valueOf(ledger.getId()) : "null"
                   + ", entryId=" + entryId
                   + ", startTime=" + startTime
                   + ", dataLength=" + dataLength
                   + '}';
       }
   ```
   
   
   
   
   


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



[GitHub] [pulsar] devinbost commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
devinbost commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-839627923


   @eolivelli 
   
   ```
   May 12 03:12:04 fab10 4e21a1596fa6: 2021-05-12T09:12:04,605 [pulsar-io-28-34] WARN  org.apache.pulsar.broker.service.ServerCnx - [/10.20.20.160:46676] Got exception java.lang.NullPointerException
   May 12 03:12:04 fab10 4e21a1596fa6: #011at org.apache.bookkeeper.mledger.impl.OpAddEntry.toString(OpAddEntry.java:354)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at org.apache.bookkeeper.mledger.impl.OpAddEntry.createOpAddEntry(OpAddEntry.java:100)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at org.apache.bookkeeper.mledger.impl.OpAddEntry.create(OpAddEntry.java:81)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl.asyncAddEntry(ManagedLedgerImpl.java:689)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at org.apache.pulsar.broker.service.persistent.PersistentTopic.asyncAddEntry(PersistentTopic.java:428)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at org.apache.pulsar.broker.service.persistent.PersistentTopic.publishMessage(PersistentTopic.java:404)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at org.apache.pulsar.broker.service.Producer.publishMessageToTopic(Producer.java:224)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at org.apache.pulsar.broker.service.Producer.publishMessage(Producer.java:161)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at org.apache.pulsar.broker.service.ServerCnx.handleSend(ServerCnx.java:1372)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at org.apache.pulsar.common.protocol.PulsarDecoder.channelRead(PulsarDecoder.java:207)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.handler.flow.FlowControlHandler.dequeue(FlowControlHandler.java:200)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.handler.flow.FlowControlHandler.channelRead(FlowControlHandler.java:162)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:324)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:296)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1504)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.handler.ssl.SslHandler.decodeNonJdkCompatible(SslHandler.java:1265)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1302)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:508)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:447)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:276)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:795)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.epoll.AbstractEpollChannel$AbstractEpollUnsafe$1.run(AbstractEpollChannel.java:425)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:164)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:472)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:384)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
   May 12 03:12:04 fab10 4e21a1596fa6: #011at java.lang.Thread.run(Thread.java:748)
   ```
   
   It looks the same to me unless I'm missing something. 
   I double checked that I built from the correct branch and that I'm using that build in my brokers. 


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



[GitHub] [pulsar] eolivelli commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-839640181


   you should move that line right after filling in the OpAddEntry, otherwise you could access recycled objects, that are in an unknown state (but probably this was your intention?)


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



[GitHub] [pulsar] linlinnn commented on a change in pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
linlinnn commented on a change in pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#discussion_r630805230



##########
File path: managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpAddEntry.java
##########
@@ -341,8 +341,8 @@ public void recycle() {
     @Override
     public String toString() {
         return "OpAddEntry{" +
-                "mlName" + ml.getName() +
-                "ledgerId=" + ledger.getId() +
+                "mlName=" + ml != null ? ml.getName() : "null" +
+                "ledgerId=" + ledger != null ? String.valueOf(ledger.getId()) : "null" +

Review comment:
       It's better to add a comma
   ```suggestion
                   ", ledgerId=" + ledger != null ? String.valueOf(ledger.getId()) : "null" +
   ```




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



[GitHub] [pulsar] devinbost edited a comment on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
devinbost edited a comment on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-839631124


   Sorry, just to clarify to prevent confusion, I'm using a build that includes this change in my fork from master that adds a lot more debug lines: https://github.com/devinbost/pulsar/tree/debug_acks
   The NPE is coming from this log line: https://github.com/devinbost/pulsar/blob/9867ecb2fa89567fc131cc7ebbd3f3371b463e75/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpAddEntry.java#L100
   


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



[GitHub] [pulsar] devinbost commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
devinbost commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-840045597


   Yep, that did the trick! Seems to work now.


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



[GitHub] [pulsar] codelipenghui merged pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548


   


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



[GitHub] [pulsar] devinbost commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
devinbost commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-839631124


   Sorry, just to clarify to prevent confusion, I'm using a build that includes this change in my fork from master that adds a lot more debug lines: https://github.com/devinbost/pulsar/tree/debug_acks
   The reason it's throwing the NPE is coming from this log line: https://github.com/devinbost/pulsar/blob/9867ecb2fa89567fc131cc7ebbd3f3371b463e75/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpAddEntry.java#L100
   


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



[GitHub] [pulsar] devinbost commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
devinbost commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-840142053


   @lhotari Looks like we have some questionable tests that are failing:
   
   > Error:  Failures: 
   > Error:  org.apache.pulsar.broker.loadbalance.LoadBalancerTest.testUpdateLoadReportAndCheckUpdatedRanking(org.apache.pulsar.broker.loadbalance.LoadBalancerTest)
   > [INFO]   Run 1: PASS
   > Error:    Run 2: LoadBalancerTest.testUpdateLoadReportAndCheckUpdatedRanking:274 expected [true] but found [false]
   > 
   > Error:  Tests run: 4, Failures: 1, Errors: 0, Skipped: 3, Time elapsed: 16.674 s <<< FAILURE! - in org.apache.pulsar.broker.service.RackAwareTest
   > Error:  testPlacement(org.apache.pulsar.broker.service.RackAwareTest)  Time elapsed: 1.919 s  <<< FAILURE!
   > org.apache.pulsar.client.admin.PulsarAdminException$ServerSideErrorException: HTTP 500 Internal Server Error
   > 	at org.apache.pulsar.client.admin.internal.BaseResource.getApiException(BaseResource.java:213)
   > 	at org.apache.pulsar.client.admin.internal.BaseResource$2.failed(BaseResource.java:152)
   > 	at org.glassfish.jersey.client.JerseyInvocation$1.failed(JerseyInvocation.java:839)
   > 	at org.glassfish.jersey.client.JerseyInvocation$1.completed(JerseyInvocation.java:820)
   > 	at org.glassfish.jersey.client.ClientRuntime.processResponse(ClientRuntime.java:229)
   > 	at org.glassfish.jersey.client.ClientRuntime.access$200(ClientRuntime.java:62)
   > 	at org.glassfish.jersey.client.ClientRuntime$2.lambda$response$0(ClientRuntime.java:173)
   > 	at org.glassfish.jersey.internal.Errors$1.call(Errors.java:248)
   > 	at org.glassfish.jersey.internal.Errors$1.call(Errors.java:244)
   > 	at org.glassfish.jersey.internal.Errors.process(Errors.java:292)
   > 	at org.glassfish.jersey.internal.Errors.process(Errors.java:274)
   > 	at org.glassfish.jersey.internal.Errors.process(Errors.java:244)
   > 	at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:288)
   > 	at org.glassfish.jersey.client.ClientRuntime$2.response(ClientRuntime.java:173)
   > 	at org.apache.pulsar.client.admin.internal.http.AsyncHttpConnector.lambda$apply$1(AsyncHttpConnector.java:212)
   > 	at java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:774)
   > 	at java.util.concurrent.CompletableFuture$UniWhenComplete.tryFire(CompletableFuture.java:750)
   > 	at java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:488)
   > 	at java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:1975)
   > 	at org.apache.pulsar.client.admin.internal.http.AsyncHttpConnector.lambda$retryOperation$4(AsyncHttpConnector.java:254)
   > 	at java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:774)
   > 	at java.util.concurrent.CompletableFuture$UniWhenComplete.tryFire(CompletableFuture.java:750)
   > 	at java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:488)
   > 	at java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:1975)
   > 	at org.asynchttpclient.netty.NettyResponseFuture.loadContent(NettyResponseFuture.java:222)
   > 	at org.asynchttpclient.netty.NettyResponseFuture.done(NettyResponseFuture.java:257)
   > 	at org.asynchttpclient.netty.handler.AsyncHttpClientHandler.finishUpdate(AsyncHttpClientHandler.java:241)
   > 	at org.asynchttpclient.netty.handler.HttpHandler.handleChunk(HttpHandler.java:114)
   > 	at org.asynchttpclient.netty.handler.HttpHandler.handleRead(HttpHandler.java:143)
   > 	at org.asynchttpclient.netty.handler.AsyncHttpClientHandler.channelRead(AsyncHttpClientHandler.java:78)
   > 	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
   > 	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
   > 	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
   > 	at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103)
   > 	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
   > 	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
   > 	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
   > 	at io.netty.channel.CombinedChannelDuplexHandler$DelegatingChannelHandlerContext.fireChannelRead(CombinedChannelDuplexHandler.java:436)
   > 	at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:324)
   > 	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:296)
   > 	at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:251)
   > 	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
   > 	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
   > 	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
   > 	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
   > 	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
   > 	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
   > 	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
   > 	at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
   > 	at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:719)
   > 	at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:655)
   > 	at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:581)
   > 	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:493)
   > 	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
   > 	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
   > 	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
   > 	at java.lang.Thread.run(Thread.java:748)
   > Caused by: javax.ws.rs.InternalServerErrorException: HTTP 500 Internal Server Error
   > 	at org.glassfish.jersey.client.JerseyInvocation.convertToException(JerseyInvocation.java:914)
   > 	at org.glassfish.jersey.client.JerseyInvocation.access$500(JerseyInvocation.java:77)
   > 	... 54 more
   > 
   > 
   > Error:  Failures: 
   > Error:  org.apache.pulsar.broker.service.MessagePublishBufferThrottleTest.testBlockByPublishRateLimiting(org.apache.pulsar.broker.service.MessagePublishBufferThrottleTest)
   > [INFO]   Run 1: PASS
   > Error:    Run 2: MessagePublishBufferThrottleTest.testBlockByPublishRateLimiting:129 expected [1] but found [0]
   
   


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



[GitHub] [pulsar] devinbost commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
devinbost commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-840142124


   /pulsarbot run-failure-checks


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



[GitHub] [pulsar] devinbost commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
devinbost commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-839637218


   I rebased both branches to ensure they're both in-sync with master now.


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



[GitHub] [pulsar] devinbost commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
devinbost commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-839530252


   Please review @lhotari @rdhabalia @codelipenghui 


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



[GitHub] [pulsar] eolivelli commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-839608254


   @devinbost can you share the new stacktrace of the error ?


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



[GitHub] [pulsar] devinbost commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
devinbost commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-839598310


   hmm this test passes:
   ```
       @Test
       public void testOpEntryAdd_toString_doesNotThrowNPE(){
           ManagedLedger ml = mock(ManagedLedger.class);
           LedgerHandle ledger = mock(LedgerHandle.class);
           when(ml.getName()).thenReturn(null);
           when(ledger.getId()).thenReturn(124L);
           long entryId = 12L;
           long startTime = 1245L;
           int dataLength = 566;
           String test = "OpAddEntry{" +
                   "mlName=" + ml != null ? ml.getName() : "null" +
                   "ledgerId=" + ledger != null ? String.valueOf(ledger.getId()) : "null" +
                   ", entryId=" + entryId +
                   ", startTime=" + startTime +
                   ", dataLength=" + dataLength +
                   '}';
       }
   ```
   
   @lhotari maybe you can spot something I'm not seeing


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



[GitHub] [pulsar] devinbost commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
devinbost commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-839583781


   I tested this, and I'm still getting an NPE on this line:
   `"mlName=" + ml != null ? ml.getName() : "null"`
   Looks like `ml.getName()` is just returning `name`...
   Any ideas?


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



[GitHub] [pulsar] devinbost commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
devinbost commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-839988053


   @lhotari Nice catch! Thanks. I'll try that. 


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



[GitHub] [pulsar] devinbost commented on pull request #10548: Made OpAddEntry.toString() more robust to nulls to prevent NPEs

Posted by GitBox <gi...@apache.org>.
devinbost commented on pull request #10548:
URL: https://github.com/apache/pulsar/pull/10548#issuecomment-840049592


   /pulsarbot run-failure-checks


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