Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skipping exceptions in RegionClient.exceptionCaught #2293

Open
vrajesh1989 opened this issue Nov 7, 2023 · 1 comment
Open

Skipping exceptions in RegionClient.exceptionCaught #2293

vrajesh1989 opened this issue Nov 7, 2023 · 1 comment

Comments

@vrajesh1989
Copy link

vrajesh1989 commented Nov 7, 2023

Hello,

We are using Opentsdb 2.4.1 and are hitting CallbackOverflowErrors and StackOverflowErrors in RegionClient.exceptionCaught. We noticed there recent patches available in master branch and are planning to cherry pick.
#839
#334
But right now we are ignoring these errors in code as follows, to avoid cleaning up of inflight RPCs to the same region server handled by the RegionClient. Could you help commenting if this is a good stop gap solution? Do you see any side effects?

public void exceptionCaught(final ChannelHandlerContext ctx,
                              final ExceptionEvent event) {
    final Throwable e = event.getCause();
    final Channel c = event.getChannel();

    if (e instanceof RejectedExecutionException) {
      LOG.warn("RPC rejected by the executor,"
               + " ignore this if we're shutting down", e);
    } else {
      LOG.error("Unexpected exception from downstream on " + c, e);
    }
    updateExceptionCounter(e.getClass());

   
    if(e.toString().contains("StackOverflowError") || e.toString().contains("CallbackOverflowError")){
      LOG.error("StackOverflowError, CallbackOverflowError thus returning error without closing channel" + e);
      return;
    }

    if (c.isOpen()) {
      Channels.close(c);  // Will trigger channelClosed(), which will cleanup()
    } else {              // else: presumably a connection timeout.
      cleanup(c);         // => need to cleanup() from here directly.
    }
  }

We are also planning to ignore ClosedChannelExceptions that we get in this method. From netty/netty#724, learnt that Netty 4.0.0 no longer throws these exceptions in exceptionCaught, so we believe ignoring them would be equivalent, instead of an upgrade to Netty 4.0.0.
Any thoughts on this?
Thanks.

@vrajesh1989
Copy link
Author

@manolama @tsuna Could you please help suggest? Thanks in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant