You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by zentol <gi...@git.apache.org> on 2018/02/28 08:44:25 UTC
[GitHub] flink pull request #5594: [FLINK-8800][REST] Reduce logging of all requests ...
GitHub user zentol opened a pull request:
https://github.com/apache/flink/pull/5594
[FLINK-8800][REST] Reduce logging of all requests to TRACE
The `AbstractHandler` is logging a DEBUG message for every received request. This can make the debug logs really noisy, so we're reducing it to TRACE.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zentol/flink 8800
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/5594.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #5594
----
commit 7660703e24690bf7027549ff928972aec4215398
Author: zentol <ch...@...>
Date: 2018-02-28T08:42:38Z
[FLINK-8800][REST] Reduce logging of all requests to TRACE
----
---
[GitHub] flink issue #5594: [FLINK-8800][REST] Reduce logging of all requests to TRAC...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5594
+1 to merge
---
[GitHub] flink pull request #5594: [FLINK-8800][REST] Reduce logging of all requests ...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/5594#discussion_r173266836
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/rest/AbstractHandler.java ---
@@ -84,8 +84,8 @@ protected AbstractHandler(
@Override
protected void respondAsLeader(ChannelHandlerContext ctx, Routed routed, T gateway) throws Exception {
- if (log.isDebugEnabled()) {
- log.debug("Received request " + routed.request().getUri() + '.');
+ if (log.isTraceEnabled()) {
--- End diff --
Yes we could simplify this, however we would be relying on implementation details of 2 libraries.
---
[GitHub] flink pull request #5594: [FLINK-8800][REST] Reduce logging of all requests ...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5594#discussion_r171196255
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/rest/AbstractHandler.java ---
@@ -84,8 +84,8 @@ protected AbstractHandler(
@Override
protected void respondAsLeader(ChannelHandlerContext ctx, Routed routed, T gateway) throws Exception {
- if (log.isDebugEnabled()) {
- log.debug("Received request " + routed.request().getUri() + '.');
+ if (log.isTraceEnabled()) {
--- End diff --
Does `routed.request().getUri()` perform some decoding work? If not, why not simply use
```java
log.trace("Received request {}.", routed.request().getUri());
```
which should be very efficient (no object creation, string concatenation, etc). Results even in less byte code.
---
[GitHub] flink pull request #5594: [FLINK-8800][REST] Reduce logging of all requests ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/5594
---
[GitHub] flink pull request #5594: [FLINK-8800][REST] Reduce logging of all requests ...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5594#discussion_r173415584
--- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/rest/AbstractHandler.java ---
@@ -84,8 +84,8 @@ protected AbstractHandler(
@Override
protected void respondAsLeader(ChannelHandlerContext ctx, Routed routed, T gateway) throws Exception {
- if (log.isDebugEnabled()) {
- log.debug("Received request " + routed.request().getUri() + '.');
+ if (log.isTraceEnabled()) {
--- End diff --
You are right. Keep it as is, to be safe.
---