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

Reply via email to