Todd Lipcon has posted comments on this change. Change subject: KUDU-1835 (part 2): enable WAL compression ......................................................................
Patch Set 8: (8 comments) http://gerrit.cloudera.org:8080/#/c/5736/8/src/kudu/consensus/log.cc File src/kudu/consensus/log.cc: PS8, Line 340: c > I forget, we don't capitalize these? yea we've been moving towards not capitalizing them, so they don't look funny when concatenated (eg "Unable to do foo: Unable to do bar: Problem with baz" isn't correct capitalization) http://gerrit.cloudera.org:8080/#/c/5736/8/src/kudu/consensus/log.proto File src/kudu/consensus/log.proto: PS8, Line 55: / Log format major/minor version. These were written by Kudu 1.2 and : // earlier, and marked as required in those versions, but unfortunately : // they were never verified on read. So, in order to make the logs written : // by newer versions give a reasonable error if an old version tries to : // read them, we no longer write these fields. : optional uint32 DEPRECATED_major_version = 1; : optional uint32 DEPRECATED_minor_version = 2; > so you're replacing these with feature flags, right? should we do the same we already did while you were on vacation, see f82cf6918c00dff6aecad5a6b50836b7eb5db508 Line 70: repeated FeatureFlag incompatible_features = 10; I discovered today that this doesn't work properly, will edit. (see KUDU-1850) http://gerrit.cloudera.org:8080/#/c/5736/8/src/kudu/consensus/log_reader.cc File src/kudu/consensus/log_reader.cc: PS8, Line 262: kEntryHeaderSizeV2 > what if this is reading an older log segment? fixed http://gerrit.cloudera.org:8080/#/c/5736/8/src/kudu/consensus/log_util.cc File src/kudu/consensus/log_util.cc: PS8, Line 429: verison > typo Done http://gerrit.cloudera.org:8080/#/c/5736/8/src/kudu/consensus/log_util.h File src/kudu/consensus/log_util.h: PS8, Line 50: Ssee > typo Done PS8, Line 305: Older versions of Kudu used a different log entry header format. > Older than what? Done http://gerrit.cloudera.org:8080/#/c/5736/8/src/kudu/util/compression/compression_codec.h File src/kudu/util/compression/compression_codec.h: PS8, Line 59: CompressionType > docs Done -- To view, visit http://gerrit.cloudera.org:8080/5736 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0fc790ddc72a519a82bbb6b71a902418051b35a5 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
