You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by asdf2014 <gi...@git.apache.org> on 2017/06/28 10:17:45 UTC
[GitHub] zookeeper pull request #296: ZOOKEEPER-2824: `FileChannel#size` info should ...
GitHub user asdf2014 opened a pull request:
https://github.com/apache/zookeeper/pull/296
ZOOKEEPER-2824: `FileChannel#size` info should be added to `FileTxnLog#commit` to solve the confuse that reason is too large log or too busy disk I/O
`FileChannel#size` info should be added to `FileTxnLog#commit` to solve the confuse that reason is too large log or too busy disk I/O
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/asdf2014/zookeeper ZOOKEEPER-2824
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/296.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 #296
----
commit 9006b8f95d9dad11ebda5f76043d973f05dcdfd2
Author: asdf2014 <15...@qq.com>
Date: 2017-06-28T10:09:09Z
ZOOKEEPER-2824: `FileChannel#size` info should be added to `FileTxnLog#commit` to solve the confuse that reason is too large log or too busy disk I/O
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper pull request #296: ZOOKEEPER-2824: `FileChannel#size` info should ...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/296#discussion_r124906442
--- Diff: src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -332,11 +333,13 @@ public synchronized void commit() throws IOException {
if (forceSync) {
long startSyncNS = System.nanoTime();
- log.getChannel().force(false);
+ FileChannel channel = log.getChannel();
+ channel.force(false);
syncElapsedMS = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startSyncNS);
if (syncElapsedMS > fsyncWarningThresholdMS) {
- LOG.warn("fsync-ing the write ahead log in "
+ LOG.warn("fsync-ing the write ahead log ("
+ + channel.size() + " bytes) in "
--- End diff --
According to the javadoc, `size()` returns "Returns the current size of this channel's file." Can you explain why this is valuable?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper issue #296: ZOOKEEPER-2824: `FileChannel#size` info should be adde...
Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/296
Hi @asdf2014
I'm happy with the change if it has any help to you for debugging.
---
[GitHub] zookeeper pull request #296: ZOOKEEPER-2824: `FileChannel#size` info should ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zookeeper/pull/296
---
[GitHub] zookeeper issue #296: ZOOKEEPER-2824: `FileChannel#size` info should be adde...
Posted by asdf2014 <gi...@git.apache.org>.
Github user asdf2014 commented on the issue:
https://github.com/apache/zookeeper/pull/296
@phunt @afine Thank you for those suggestions. Considered very thoughtful. Already fixed.
---
[GitHub] zookeeper pull request #296: ZOOKEEPER-2824: `FileChannel#size` info should ...
Posted by phunt <gi...@git.apache.org>.
Github user phunt commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/296#discussion_r162510193
--- Diff: src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -332,11 +333,13 @@ public synchronized void commit() throws IOException {
if (forceSync) {
long startSyncNS = System.nanoTime();
- log.getChannel().force(false);
+ FileChannel channel = log.getChannel();
+ channel.force(false);
syncElapsedMS = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startSyncNS);
if (syncElapsedMS > fsyncWarningThresholdMS) {
- LOG.warn("fsync-ing the write ahead log in "
+ LOG.warn("fsync-ing the write ahead log ("
+ + channel.size() + " bytes) in "
--- End diff --
@asdf2014 can you move the changed text after the first sentence?
... latency. File size is ### bytes. See ...
The reason being that I know a number of users who have log triggers on this log message and I'd like to minimize the impact as much as possible.
---
[GitHub] zookeeper issue #296: ZOOKEEPER-2824: `FileChannel#size` info should be adde...
Posted by asdf2014 <gi...@git.apache.org>.
Github user asdf2014 commented on the issue:
https://github.com/apache/zookeeper/pull/296
@afine You are welcome :-)
---
[GitHub] zookeeper pull request #296: ZOOKEEPER-2824: `FileChannel#size` info should ...
Posted by asdf2014 <gi...@git.apache.org>.
Github user asdf2014 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/296#discussion_r124950647
--- Diff: src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -332,11 +333,13 @@ public synchronized void commit() throws IOException {
if (forceSync) {
long startSyncNS = System.nanoTime();
- log.getChannel().force(false);
+ FileChannel channel = log.getChannel();
+ channel.force(false);
syncElapsedMS = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startSyncNS);
if (syncElapsedMS > fsyncWarningThresholdMS) {
- LOG.warn("fsync-ing the write ahead log in "
+ LOG.warn("fsync-ing the write ahead log ("
+ + channel.size() + " bytes) in "
--- End diff --
@afine Thank you for code review. When we got the warn message, will confuse that reason is `too large log` or `too busy disk I/O`? So, need to show the size of log to eliminate the `too large log` possibility.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zookeeper issue #296: ZOOKEEPER-2824: `FileChannel#size` info should be adde...
Posted by asdf2014 <gi...@git.apache.org>.
Github user asdf2014 commented on the issue:
https://github.com/apache/zookeeper/pull/296
Hi, @anmolnar . Thanks, would you approve this PR?
---
[GitHub] zookeeper issue #296: ZOOKEEPER-2824: `FileChannel#size` info should be adde...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on the issue:
https://github.com/apache/zookeeper/pull/296
Merged. Thanks @asdf2014!
---
[GitHub] zookeeper pull request #296: ZOOKEEPER-2824: `FileChannel#size` info should ...
Posted by afine <gi...@git.apache.org>.
Github user afine commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/296#discussion_r164878614
--- Diff: src/java/main/org/apache/zookeeper/server/persistence/FileTxnLog.java ---
@@ -332,11 +333,13 @@ public synchronized void commit() throws IOException {
if (forceSync) {
long startSyncNS = System.nanoTime();
- log.getChannel().force(false);
+ FileChannel channel = log.getChannel();
+ channel.force(false);
syncElapsedMS = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startSyncNS);
if (syncElapsedMS > fsyncWarningThresholdMS) {
- LOG.warn("fsync-ing the write ahead log in "
+ LOG.warn("fsync-ing the write ahead log ("
+ + channel.size() + " bytes) in "
--- End diff --
@asdf2014 as soon as @phunt's comment is addressed I can merge this in.
---
[GitHub] zookeeper issue #296: ZOOKEEPER-2824: `FileChannel#size` info should be adde...
Posted by asdf2014 <gi...@git.apache.org>.
Github user asdf2014 commented on the issue:
https://github.com/apache/zookeeper/pull/296
@afine Could you approve this PR?
@phunt @anmolnar PTAL.

Just recently, i experienced this situation again, this time due to the bad disk rather than the huge WAL file. But i still cannot get the root cause from Zookeeper log...
---