Adar Dembo has posted comments on this change.
Change subject: tool: port log-dump
Patch Set 3:
PS3, Line 92: out
> Some basic paranoia Qn unrelated to your change: The string object itself s
The string _data_ itself is heap-allocated; std::string is just a small
Line 369: const Schema kSchemaWithIds(SchemaBuilder(kSchema).Build());
> I am curious to know the purpose of SchemaBuilder here ? Can't we directly
Apparently not. I copied this from log-test-base.h because when I tried to use
kSchema as-is in Log::Open(), I got a log that could not be read later on. I
guess you need to rebuild the schema with column IDs like this. I agree that
it's unintuitive; I was just trying to "get it to work".
PS3, Line 265: AddOptionalParameter
> I have been wondering about this while testing today: isn't fs_wal_dir supp
fs_wal_dir and fs_data_dirs are "optional" in the sense that they're gflags and
thus can be omitted from the command line and the tool still run. However,
FsManager::Open will fail if their values aren't provided.
This is confusing because all other gflags are optional. For a while I thought
maybe we should add required (positional) parameters for these two, then map
their values to gflags values when parsing. But then the number of positional
parameters is quite high, and that's annoying to deal with (i.e. it's easy to
get them out of order on the command line).
So in a nutshell, I'm not really sure how we should handle them. We could make
it explicit that some keyval args (i.e. --fs_wal_dir) are required, but that's
To view, visit http://gerrit.cloudera.org:8080/4167
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Owner: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>