Mike Percy has posted comments on this change. Change subject: KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata ......................................................................
Patch Set 15: (1 comment) http://gerrit.cloudera.org:8080/#/c/3823/15/src/kudu/tablet/tablet_metadata.cc File src/kudu/tablet/tablet_metadata.cc: PS15, Line 313: DCHECK_EQ(table_id_, superblock.table_id()); : PartitionSchemaPB partition_schema_pb; : partition_schema_.ToPB(&partition_schema_pb); : DCHECK_EQ(superblock.partition_schema().SerializeAsString(), : partition_schema_pb.SerializeAsString()); : PartitionPB partition_pb; : partition_.ToPB(&partition_pb); : DCHECK_EQ(superblock.partition().SerializeAsString(), : partition_pb.SerializeAsString()); > 1. I actually thought about NDEBUG, but forgot to materialize in between th #1 is a good idea Regarding #2, since it's a DCHECK we ignore it in release mode. Below is what the PBs look like. I suppose we could create equality functions for them to try to future proof them from PB changes but I'm not sure it's worth it for a DCHECK. message PartitionPB { // The hash buckets of the partition. The number of hash buckets must match // the number of hash bucket components in the partition's schema. repeated int32 hash_buckets = 1 [packed = true]; // The encoded start partition key (inclusive). optional bytes partition_key_start = 2; // The encoded end partition key (exclusive). optional bytes partition_key_end = 3; } // The serialized format of a Kudu table partition schema. message PartitionSchemaPB { // A column identifier for partition schemas. In general, the name will be // used when a client creates the table since column IDs are assigned by the // master. All other uses of partition schemas will use the numeric column ID. message ColumnIdentifierPB { oneof identifier { int32 id = 1; string name = 2; } } message RangeSchemaPB { // Column identifiers of columns included in the range. All columns must be // a component of the primary key. repeated ColumnIdentifierPB columns = 1; } message HashBucketSchemaPB { // Column identifiers of columns included in the hash. Every column must be // a component of the primary key. repeated ColumnIdentifierPB columns = 1; // Number of buckets into which columns will be hashed. Must be at least 2. required int32 num_buckets = 2; // Seed value for hash calculation. Administrators may set a seed value // on a per-table basis in order to randomize the mapping of rows to // buckets. Setting a seed provides some amount of protection against denial // of service attacks when the hash bucket columns contain user provided // input. optional uint32 seed = 3; enum HashAlgorithm { UNKNOWN = 0; MURMUR_HASH_2 = 1; } // The hash algorithm to use for calculating the hash bucket. optional HashAlgorithm hash_algorithm = 4; } repeated HashBucketSchemaPB hash_bucket_schemas = 1; optional RangeSchemaPB range_schema = 2; } -- To view, visit http://gerrit.cloudera.org:8080/3823 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If57617e22b41296b8d4e8ad131220f1ebb235019 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat <din...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Dan Burkert <d...@cloudera.com> Gerrit-Reviewer: Dinesh Bhat <din...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes