You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "William Li (Code Review)" <ge...@cloudera.org> on 2017/05/11 02:01:30 UTC
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
William Li has uploaded a new change for review.
http://gerrit.cloudera.org:8080/6853
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
KUDU-1192 Periodically flush glog buffers from a thread
Added a timer inside the AsyncLogger to periodically calls the flush().
It wakes up the main RunThread() to flush the messages from buffers
Period to wake up is default to 1 second but can also use environment variable to change it
Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/async_logger.h
M src/kudu/util/logging-test.cc
3 files changed, 113 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/6853/1
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
Patch Set 5:
(3 comments)
Can you also add something like:
google::SetCommandLineOptionWithMode("logbufsecs", "5",
google::FlagSettingMode::SET_FLAGS_DEFAULT);
in ParseCommandLineFlags() in flags.cc? I think changing our default to 5 secs instead of 30 is a good idea with this change, so that the messages are flushed a bit quicker.
http://gerrit.cloudera.org:8080/#/c/6853/5/src/kudu/util/async_logger.cc
File src/kudu/util/async_logger.cc:
PS5, Line 120: regardless there
"regardless whether there is"
http://gerrit.cloudera.org:8080/#/c/6853/5/src/kudu/util/logging-test.cc
File src/kudu/util/logging-test.cc:
Line 175: // wait a little more than one wake up cycle
nit: capitalize sentence and end in '.'
nit: "wake-up"
PS5, Line 180: flush
nit: flushed
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6853/6/src/kudu/util/logging-test.cc
File src/kudu/util/logging-test.cc:
PS6, Line 180: // the AsyncLogger should have flushed at least once by the timer automatically
: // so there should be no more messages in the buffer.
This may not be the case if the system was under a lot of load at the time of SleepFor() (i.e. it's possible for the sleep to finish before the async logger thread was scheduled).
Rather than hardcode a particular sleep value, wrap the assertions you want to make in an ASSERT_EVENTUALLY() block. Something like this:
async.Write(...);
async.Write(...);
ASSERT_EVENTUALLY([&]() {
ASSERT_EQ(base.message_count_, 2);
ASSERT_GT(base.flush_count_, 0);
});
async.Stop();
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6853
to look at the new patch set (#11).
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
KUDU-1192 Periodically flush glog buffers from a thread
Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
When the wait times out, it will flush even if there is nothing enqueued.
Also sets the logbufsecs default to 5 seconds, instead of the 30s.
Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/flags.cc
M src/kudu/util/logging-test.cc
3 files changed, 35 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/6853/11
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6853
to look at the new patch set (#5).
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
KUDU-1192 Periodically flush glog buffers from a thread
Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
When the wait times out, it will flush even if there is nothing enqueued.
Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/logging-test.cc
2 files changed, 27 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/6853/5
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6853
to look at the new patch set (#7).
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
KUDU-1192 Periodically flush glog buffers from a thread
Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
When the wait times out, it will flush even if there is nothing enqueued.
Also sets the logbufsecs default to 5 seconds, instead of the 30s.
Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/flags.cc
M src/kudu/util/logging-test.cc
3 files changed, 31 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/6853/7
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6853/1/src/kudu/util/async_logger.cc
File src/kudu/util/async_logger.cc:
Line 34: const char* kAsyncLoggerTimerEnvVar = "KUDU_ASYNC_LOGGER_TIMER";
rather than using an env var, should use a gflag for the period at which this is flushed. We can probably just reuse the existing FLAGS_logbufsecs in fact
Line 54: timer_thread_ = std::thread(&AsyncLogger::RunTimer, this);
instead of a separate thread, why not just change the existing thread so that instead of doing a Wait() with no timeout, it does a Wait() with a timeout of logbufsecs and then wakes up and flushes even if nothing has been enqueued?
Line 148: wake_flusher_cond_.Wait();
eg here you can use a TimedWait, and if it times out, set active_buf_->flush = true
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
William Li has posted comments on this change.
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6853/3/src/kudu/util/async_logger.cc
File src/kudu/util/async_logger.cc:
Line 121: active_buf_->flush = true;
> we only want to do this if we haven't flushed in the configured amount of t
Done
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6853/7/src/kudu/util/logging-test.cc
File src/kudu/util/logging-test.cc:
Line 176: ASSERT_EVENTUALLY([&]() {
> error: use of undeclared identifier 'ASSERT_EVENTUALLY' [clang-diagnostic-e
think you need to include test_util or test_macros or something (forget where this is defined)
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6853
to look at the new patch set (#4).
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
KUDU-1192 Periodically flush glog buffers from a thread
Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
When wait times out, it will flush even if there is nothing enqueued.
Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/logging-test.cc
2 files changed, 27 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/6853/4
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6853
to look at the new patch set (#10).
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
KUDU-1192 Periodically flush glog buffers from a thread
Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
When the wait times out, it will flush even if there is nothing enqueued.
Also sets the logbufsecs default to 5 seconds, instead of the 30s.
Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/flags.cc
M src/kudu/util/logging-test.cc
3 files changed, 34 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/6853/10
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
William Li has posted comments on this change.
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
Patch Set 6:
(2 comments)
sorry my bad. included the test_util.h
http://gerrit.cloudera.org:8080/#/c/6853/7/src/kudu/util/logging-test.cc
File src/kudu/util/logging-test.cc:
Line 176: SleepFor(MonoDelta::FromSeconds(FLAGS_logbufsecs + 1));
> think you need to include test_util or test_macros or something (forget whe
Done
Line 176: SleepFor(MonoDelta::FromSeconds(FLAGS_logbufsecs + 1));
> error: use of undeclared identifier 'ASSERT_EVENTUALLY' [clang-diagnostic-e
Done
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6853
to look at the new patch set (#2).
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
KUDU-1192 Periodically flush glog buffers from a thread
Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
Either gets a signal or wait timeout, it will continue the thread to flush the messages from buffers
Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/logging-test.cc
2 files changed, 27 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/6853/2
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6853/3/src/kudu/util/async_logger.cc
File src/kudu/util/async_logger.cc:
Line 121: // In case of wait timeout, force it to flush regardless there is anything enqueued.
we only want to do this if we haven't flushed in the configured amount of time. The way you have it now, we are now flushing every log message.
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
KUDU-1192 Periodically flush glog buffers from a thread
Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
When the wait times out, it will flush even if there is nothing enqueued.
Also sets the logbufsecs default to 5 seconds, instead of the 30s.
Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Reviewed-on: http://gerrit.cloudera.org:8080/6853
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/util/async_logger.cc
M src/kudu/util/flags.cc
M src/kudu/util/logging-test.cc
3 files changed, 35 insertions(+), 3 deletions(-)
Approvals:
Adar Dembo: Looks good to me, but someone else must approve
Todd Lipcon: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6853
to look at the new patch set (#9).
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
KUDU-1192 Periodically flush glog buffers from a thread
Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
When the wait times out, it will flush even if there is nothing enqueued.
Also sets the logbufsecs default to 5 seconds, instead of the 30s.
Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/flags.cc
M src/kudu/util/logging-test.cc
3 files changed, 32 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/6853/9
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6853
to look at the new patch set (#6).
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
KUDU-1192 Periodically flush glog buffers from a thread
Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
When the wait times out, it will flush even if there is nothing enqueued.
Also sets the logbufsecs default to 5 seconds, instead of the 30s.
Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/flags.cc
M src/kudu/util/logging-test.cc
3 files changed, 31 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/6853/6
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6853
to look at the new patch set (#3).
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
KUDU-1192 Periodically flush glog buffers from a thread
Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
Either gets a signal or wait timeout, it will continue the thread run
and to flush the messages from buffers
Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/logging-test.cc
2 files changed, 27 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/6853/3
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
William Li has posted comments on this change.
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6853/6/src/kudu/util/logging-test.cc
File src/kudu/util/logging-test.cc:
PS6, Line 180: // the AsyncLogger should have flushed at least once by the timer automatically
: // so there should be no more messages in the buffer.
> This may not be the case if the system was under a lot of load at the time
Done
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
William Li has posted comments on this change.
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
Patch Set 11:
(14 comments)
all are related to the missing <atomic> header file. fixed.
http://gerrit.cloudera.org:8080/#/c/6853/10/src/kudu/util/logging-test.cc
File src/kudu/util/logging-test.cc:
Line 103: int /*message_len*/) override {
> error: use of undeclared identifier 'message_count_' [clang-diagnostic-erro
Done
Line 112: SleepFor(MonoDelta::FromMilliseconds(5));
> error: use of undeclared identifier 'flush_count_' [clang-diagnostic-error]
Done
Line 119:
> error: expected member name or ';' after declaration specifiers [clang-diag
Done
Line 119:
> error: no type named 'atomic' in namespace 'std' [clang-diagnostic-error]
Done
Line 120: std::atomic<int> flush_count_ = {0};
> error: no type named 'atomic' in namespace 'std' [clang-diagnostic-error]
Done
Line 120: std::atomic<int> flush_count_ = {0};
> error: expected member name or ';' after declaration specifiers [clang-diag
Done
Line 156: async.Stop();
> error: no member named 'message_count_' in 'kudu::CountingLogger' [clang-di
Done
Line 156: async.Stop();
> error: no type named 'Compare' in the global namespace [clang-diagnostic-er
Done
Line 156: async.Stop();
> error: expected ')' [clang-diagnostic-error]
Done
Line 160: // 'flush' set to true.
> error: no member named 'flush_count_' in 'kudu::CountingLogger' [clang-diag
Done
Line 178: ASSERT_EVENTUALLY([&]() {
> error: no type named 'Compare' in the global namespace [clang-diagnostic-er
Done
Line 178: ASSERT_EVENTUALLY([&]() {
> error: expected ')' [clang-diagnostic-error]
Done
Line 178: ASSERT_EVENTUALLY([&]() {
> error: no member named 'message_count_' in 'kudu::CountingLogger' [clang-di
Done
Line 181: // so there should be no more messages in the buffer.
> error: no member named 'flush_count_' in 'kudu::CountingLogger' [clang-diag
Done
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
Patch Set 11: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
Gerrit-HasComments: No
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6853
to look at the new patch set (#8).
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
KUDU-1192 Periodically flush glog buffers from a thread
Added a wait timeout to AsyncLogger RunThread based on FLAGS_logbufsec
When the wait times out, it will flush even if there is nothing enqueued.
Also sets the logbufsecs default to 5 seconds, instead of the 30s.
Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
---
M src/kudu/util/async_logger.cc
M src/kudu/util/flags.cc
M src/kudu/util/logging-test.cc
3 files changed, 32 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/6853/8
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
William Li has posted comments on this change.
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
Patch Set 6:
(3 comments)
done the fixes. Also added the SetCommandLineOptionWithMode per suggestion, although I am not sure if additional tests need to be written for that. Seem there are already good tests for the SetCommandLineOptionWithMode already. So if need to add our own tests, please let me know.
http://gerrit.cloudera.org:8080/#/c/6853/5/src/kudu/util/async_logger.cc
File src/kudu/util/async_logger.cc:
PS5, Line 120: regardless wheth
> "regardless whether there is"
Done
http://gerrit.cloudera.org:8080/#/c/6853/5/src/kudu/util/logging-test.cc
File src/kudu/util/logging-test.cc:
Line 175: // Wait a little more than one wake-up cycle.
> nit: capitalize sentence and end in '.'
Done
PS5, Line 180: flush
> nit: flushed
Done
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1192 Periodically flush glog buffers from a thread
Posted by "William Li (Code Review)" <ge...@cloudera.org>.
William Li has posted comments on this change.
Change subject: KUDU-1192 Periodically flush glog buffers from a thread
......................................................................
Patch Set 2:
(2 comments)
removed the changes from other files as well.
http://gerrit.cloudera.org:8080/#/c/6853/1/src/kudu/util/async_logger.cc
File src/kudu/util/async_logger.cc:
Line 34: wrapped_(DCHECK_NOTNULL(wrapped)),
> rather than using an env var, should use a gflag for the period at which th
yup. make things much simplier?
Line 54: CHECK_EQ(state_, RUNNING);
> instead of a separate thread, why not just change the existing thread so th
Done
--
To view, visit http://gerrit.cloudera.org:8080/6853
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id4c6d440e9259efcf222530f13137f7de5bf00fc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: William Li <wi...@inspur.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: William Li <wi...@inspur.com>
Gerrit-HasComments: Yes