David Ribeiro Alves has posted comments on this change. Change subject: KUDU-1835 (part 2): enable WAL compression ......................................................................
Patch Set 8: (7 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? 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 for cfiles? 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? 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 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 PS8, Line 305: Older versions of Kudu used a different log entry header format. Older than what? 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 -- 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-HasComments: Yes
