NIFI-8405: Added debug logging around how long it takes to establish …#4983
NIFI-8405: Added debug logging around how long it takes to establish …#4983markap14 wants to merge 2 commits intoapache:mainfrom
Conversation
…connections/query dns/read and write headers and body when replication requests; added additional timing around Ranger audits and authorizations and monitoring of long-running tasks because those run often and frequently show up in the logs at the same time as the long requests
| okHttpClientBuilder.followRedirects(true); | ||
| final int connectionPoolSize = properties.getClusterNodeMaxConcurrentRequests(); | ||
| okHttpClientBuilder.connectionPool(new ConnectionPool(connectionPoolSize, 5, TimeUnit.MINUTES)); | ||
| okHttpClientBuilder.eventListener(new RequestReplicationEventListener()); |
There was a problem hiding this comment.
Instead of introducing the custom RequestReplicationEventListener and CallEventListener classes for logging, would the okhttp3.logging.LoggingEventListener class accomplish the same goal?
Introducing the potential for logging events could have performance implications, what do you think about adding check for logger.isDebugEnabled() to control whether or not to add the Event Listener?
There was a problem hiding this comment.
I took a look at the LoggingEventListener, but it logs each individual event individually, and since we're always replicating to each node in the cluster, that becomes really difficult to partition out and understand, either programmatically or manually.
I did also consider using a mechanism to avoid the expensive of doing this if logging is not enabled. But since this is done only when we replicate requests across the cluster, that's not terribly frequently and so the overhead is almost nil. We could, in the future, improve it, by checking isDebugEnabled() but I didn't want to cloud the code at this point, given the (little) expense of the operation and how infrequently it's performed. Plus, I can envision improving this in the future to do something like always warn if DNS lookup takes more than 1 second or something like that, even if debug logging is not enabled.
|
|
||
| getLogger().info("Active threads: {}; Long running threads: {}", activeThreadCount, longRunningThreadCount); | ||
| final long nanos = System.nanoTime() - start; | ||
| getLogger().info("Active threads: {}; Long running threads: {}; time to check: {} nanos", activeThreadCount, longRunningThreadCount, NumberFormat.getInstance().format(nanos)); |
There was a problem hiding this comment.
Unfortunately, changing this logging statement broke the existing unit test, so some adjustment will be necessary.
|
Thanks for the responses @markap14, the implementation seems reasonable given the goals described. The unit test update looks good. +1 Merging. |
…connections/query dns/read and write headers and body when replication requests; added additional timing around Ranger audits and authorizations and monitoring of long-running tasks because those run often and frequently show up in the logs at the same time as the long requests This closes apache#4983 Signed-off-by: David Handermann <exceptionfactory@apache.org>
2 participants
…connections/query dns/read and write headers and body when replication requests; added additional timing around Ranger audits and authorizations and monitoring of long-running tasks because those run often and frequently show up in the logs at the same time as the long requests
Thank you for submitting a contribution to Apache NiFi.
Please provide a short description of the PR here:
Description of PR
Enables X functionality; fixes bug NIFI-YYYY.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically
main)?Is your initial contribution a single, squashed commit? Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not
squashor use--forcewhen pushing to allow for clean monitoring of changes.For code changes:
mvn -Pcontrib-check clean installat the rootnififolder?LICENSEfile, including the mainLICENSEfile undernifi-assembly?NOTICEfile, including the mainNOTICEfile found undernifi-assembly?.displayNamein addition to .name (programmatic access) for each of the new properties?For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI for build issues and submit an update to your PR as soon as possible.