Fix unsupported request type error handling and catch exception #2582
Conversation
| @@ -37,6 +36,7 @@ public class NettyServerRequestResponseChannel implements RequestResponseChannel | |||
| private final NetworkRequestQueue networkRequestQueue; | |||
| private final ServerMetrics serverMetrics; | |||
| private final ServerRequestResponseHelper requestResponseHelper; | |||
| protected static final Logger publicAccessLogger = LoggerFactory.getLogger("PublicAccessLogger"); | |||
There was a problem hiding this comment.
I'm not sure how the LoggerFactory works. With that, will publicAccessLogger write to the public log instead of the ambry log?
Also, right now we are using LogManager instead of LoggerFactory for the PublicAccessLogger. I don't know the difference between them.
private static final Logger publicAccessLogger = LogManager.getLogger("PublicAccessLogger");
There was a problem hiding this comment.
Yeah, I think the Logger obtained via PublicAccessLogger name will write logs to public access logs. I think log4j2 has concept of "appenders" which allows us to choose the destination for a logger via configuration. If we see in internal AmbryLI configuration, the appender for PublicAccessLogger redirects logs to public access logs.
LogManager is used for structured logging. But I think we only use it for frontends. For servers, we are still using LoggerFactory. All the server public access logging happens in AmbryRequests.java.
This is my understanding :)
| sendResponse(response, networkRequest, null); | ||
| } catch (IOException | InterruptedException e) { | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
we should capture Throwable, instead of Exception.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #2582 +/- ##
=============================================
- Coverage 72.51% 32.76% -39.76%
+ Complexity 11375 4458 -6917
=============================================
Files 803 803
Lines 64660 64684 +24
Branches 7897 7897
=============================================
- Hits 46890 21191 -25699
- Misses 15203 41260 +26057
+ Partials 2567 2233 -334
☔ View full report in Codecov by Sentry. |
Catch exception in
NettyRequestResponseChannel.rejectRequestsso that it is not propagated to request handler threads and cause server shutdown