Adar Dembo has posted comments on this change.
Change subject: catalog_manager: don't log deleted tables/tablets at startup
......................................................................
Patch Set 2:
(8 comments)
> How confident are we that the Substitutes won't be more eagerly
> evaluated than the << operations?
We discussed this offline. I wrote a test that evaluated this code:
VLOG(1) << Substitute("abcd $0", [&]() {
CHECK(false);
return string("12345");
}());
When run normally, it passed. When run with -v=2, it aborted.
So it appears that glog is lazily evaluating the entirety of the expression
after a VLOG.
http://gerrit.cloudera.org:8080/#/c/7826/2//COMMIT_MSG
Commit Message:
PS2, Line 14: overriden
> nit: overridden
Done
http://gerrit.cloudera.org:8080/#/c/7826/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
Line 1308: LOG(INFO) << Substitute("Servicing CreateTable request from $0:
$1",
> The previous version has a newline before the table descriptor. I've alway
I did this to be consistent with AlterTable/DeleteTable, but I don't mind a
newline for all three.
Line 2357: VLOG(2) << Substitute("Received tablet report from $0: $1",
> Might be good to put a leading newline on the tablet report here for the sa
Done
PS1, Line 3133: deleted
> I think it was correct before
Whoops.
http://gerrit.cloudera.org:8080/#/c/7826/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
PS2, Line 359: VLOG(2)
> interesting: for tables the debug log level is 1. Is this intentional to h
That's the way the old code was; can't say for sure since I didn't write it (I
don't think).
But I'll put them at the same log level.
PS2, Line 2616: WARNING
> Maybe, it's worth set to up ERROR since this is an error condition and the
Done
PS2, Line 2951: ...
> nit: drop the ellipsis?
Done
PS2, Line 2947: if (delay_millis <= 0) {
: LOG(WARNING) << "Request timed out: " << description();
: MarkFailed();
: } else {
: LOG(INFO) << Substitute("Scheduling retry of $0 with a
delay of $1 ms (attempt = $2)...",
: description(), delay_millis,
attempt_);
: master_->messenger()->ScheduleOnReactor(
: boost::bind(&RetryingTSRpcTask::RunDelayedTask, this,
_1),
: MonoDelta::FromMilliseconds(delay_millis));
: return true;
: }
: return false;
> nit: while you are at it, consider removing the extra-scope for better read
Done
--
To view, visit http://gerrit.cloudera.org:8080/7826
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I557ebfe9d3ce1663e7f7c189f0087bdd29a09306
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes
