From 6d09837b50613b8f4ee3a84f41faf2e055808d2f Mon Sep 17 00:00:00 2001 From: Leonid Koftun Date: Mon, 3 Feb 2020 23:20:59 +0100 Subject: [PATCH 01/20] Add alter partition reassignments API --- admin.go | 44 +++++++ alter_partition_reassignments_request.go | 111 ++++++++++++++++++ alter_partition_reassignments_response.go | 135 ++++++++++++++++++++++ broker.go | 13 +++ packet_encoder.go | 1 + prep_encoder.go | 5 + real_encoder.go | 5 + request.go | 2 + 8 files changed, 316 insertions(+) create mode 100644 alter_partition_reassignments_request.go create mode 100644 alter_partition_reassignments_response.go diff --git a/admin.go b/admin.go index dd634846d..40e805608 100644 --- a/admin.go +++ b/admin.go @@ -42,6 +42,13 @@ type ClusterAdmin interface { // new partitions. This operation is supported by brokers with version 1.0.0 or higher. CreatePartitions(topic string, count int32, assignment [][]int32, validateOnly bool) error + // Update the configuration for the specified resources with the default options. + // This operation is supported by brokers with version 0.11.0.0 or higher. + // The resources with their configs (topic is the only resource type with configs + // that can be updated currently Updates are not transactional so they may succeed + // for some resources while fail for others. The configs for a particular resource are updated automatically. + AlterPartitionReassignments(topic string, assignment [][]int32) error + // Delete records whose offset is smaller than the given offset of the corresponding partition. // This operation is supported by brokers with version 0.11.0.0 or higher. DeleteRecords(topic string, partitionOffsets map[int32]int64) error @@ -443,6 +450,43 @@ func (ca *clusterAdmin) CreatePartitions(topic string, count int32, assignment [ }) } +func (ca *clusterAdmin) AlterPartitionReassignments(topic string, assignment [][]int32) error { + if topic == "" { + return ErrInvalidTopic + } + + request := &AlterPartitionReassignmentsRequest{ + TimeoutMs: int32(10000), + Version: int16(0), + } + + for i := 0; i < len(assignment); i++ { + for j := 0; j < len(assignment[i]); j++ { + // TODO + request.AddBlock(topic, int32(i), assignment[i][j]) + } + } + + return ca.retryOnError(isErrNoController, func() error { + b, err := ca.Controller() + if err != nil { + return err + } + + rsp, err := b.AlterPartitionReassignments(request) + + _ = rsp + + if err != nil { + return err + } + + // TODO error handling + + return nil + }) +} + func (ca *clusterAdmin) DeleteRecords(topic string, partitionOffsets map[int32]int64) error { if topic == "" { return ErrInvalidTopic diff --git a/alter_partition_reassignments_request.go b/alter_partition_reassignments_request.go new file mode 100644 index 000000000..2392885ce --- /dev/null +++ b/alter_partition_reassignments_request.go @@ -0,0 +1,111 @@ +package sarama + +type alterPartitionReassignmentsBlock struct { + replicas int32 +} + +func (b *alterPartitionReassignmentsBlock) encode(pe packetEncoder, version int16) error { + pe.putInt32(b.replicas) + return nil +} + +func (b *alterPartitionReassignmentsBlock) decode(pd packetDecoder, version int16) (err error) { + b.replicas, err = pd.getInt32() + return err +} + +type AlterPartitionReassignmentsRequest struct { + TimeoutMs int32 + blocks map[string]map[int32]*alterPartitionReassignmentsBlock + Version int16 +} + +func (r *AlterPartitionReassignmentsRequest) encode(pe packetEncoder) error { + + pe.putInt32(r.TimeoutMs) + + if err := pe.putArrayLength(len(r.blocks)); err != nil { + return err + } + + for topic, partitions := range r.blocks { + if err := pe.putCompactString(topic); err != nil { + return err + } + if err := pe.putArrayLength(len(partitions)); err != nil { + return err + } + for partition, block := range partitions { + pe.putInt32(partition) + if err := block.encode(pe, r.Version); err != nil { + return err + } + } + } + return nil +} + +func (r *AlterPartitionReassignmentsRequest) decode(pd packetDecoder, version int16) (err error) { + r.Version = version + + if r.TimeoutMs, err = pd.getInt32(); err != nil { + return err + } + + topicCount, err := pd.getArrayLength() + if err != nil { + return err + } + if topicCount == 0 { + return nil + } + r.blocks = make(map[string]map[int32]*alterPartitionReassignmentsBlock) + for i := 0; i < topicCount; i++ { + topic, err := pd.getString() + if err != nil { + return err + } + partitionCount, err := pd.getArrayLength() + if err != nil { + return err + } + r.blocks[topic] = make(map[int32]*alterPartitionReassignmentsBlock) + for j := 0; j < partitionCount; j++ { + partition, err := pd.getInt32() + if err != nil { + return err + } + block := &alterPartitionReassignmentsBlock{} + if err := block.decode(pd, r.Version); err != nil { + return err + } + r.blocks[topic][partition] = block + } + } + + return +} + +func (r *AlterPartitionReassignmentsRequest) key() int16 { + return 45 +} + +func (r *AlterPartitionReassignmentsRequest) version() int16 { + return r.Version +} + +func (r *AlterPartitionReassignmentsRequest) requiredVersion() KafkaVersion { + return MinVersion +} + +func (r *AlterPartitionReassignmentsRequest) AddBlock(topic string, partitionID int32, replicas int32) { + if r.blocks == nil { + r.blocks = make(map[string]map[int32]*alterPartitionReassignmentsBlock) + } + + if r.blocks[topic] == nil { + r.blocks[topic] = make(map[int32]*alterPartitionReassignmentsBlock) + } + + r.blocks[topic][partitionID] = &alterPartitionReassignmentsBlock{replicas} +} diff --git a/alter_partition_reassignments_response.go b/alter_partition_reassignments_response.go new file mode 100644 index 000000000..b3aef4694 --- /dev/null +++ b/alter_partition_reassignments_response.go @@ -0,0 +1,135 @@ +package sarama + +//type alterPartitionReassignmentsErrorBlock struct { +// errorCode KError +// errorMessage string +//} +// +//func (b *alterPartitionReassignmentsErrorBlock) encode(pe packetEncoder) error { +// pe.putInt32(b.errorCode) +// pe.putString(b.errorMessage) +// +// return nil +//} +// +//func (b *alterPartitionReassignmentsErrorBlock) decode(pd packetDecoder) (err error) { +// if b.errorCode, err = pd.getInt32(); err != nil { +// return err +// } +// b.errorMessage, err = pd.getString() +// return err +//} + +type AlterPartitionReassignmentsResponse struct { + Version int16 + ThrottleTimeMs int32 + ErrorCode KError + ErrorMessage *string + //Errors map[string]map[int32]*alterPartitionReassignmentsErrorBlock +} + +//func (r *AlterPartitionReassignmentsResponse) AddError(topic string, partition int32, kerror KError) { +// // TODO +// if r.Errors == nil { +// r.Errors = make(map[string]map[int32]KError) +// } +// partitions := r.Errors[topic] +// if partitions == nil { +// partitions = make(map[int32]KError) +// r.Errors[topic] = partitions +// } +// partitions[partition] = kerror +//} + +func (r *AlterPartitionReassignmentsResponse) encode(pe packetEncoder) error { + pe.putInt32(r.ThrottleTimeMs) + //pe.putInt16(int16(r.ErrorCode)) + //if err := pe.putNullableString(r.ErrorMessage); err != nil { + // return err + //} + + //if err := pe.putArrayLength(len(r.Errors)); err != nil { + // return err + //} + //for topic, partitions := range r.Errors { + // if err := pe.putString(topic); err != nil { + // return err + // } + // if err := pe.putArrayLength(len(partitions)); err != nil { + // return err + // } + // for partition, kerror := range partitions { + // pe.putInt32(partition) + // pe.putInt16(int16(kerror)) + // } + //} + return nil +} + +func (r *AlterPartitionReassignmentsResponse) decode(pd packetDecoder, version int16) (err error) { + r.Version = version + + if r.ThrottleTimeMs, err = pd.getInt32(); err != nil { + return err + } + + //kerr, err := pd.getInt16(); + //if err != nil { + // return err + //} + // + //r.ErrorCode = KError(kerr) + // + //if r.ErrorMessage, err = pd.getNullableString(); err != nil { + // return err + //} + + return nil + + //numTopics, err := pd.getArrayLength() + //if err != nil || numTopics == 0 { + // return err + //} + // + //r.Errors = make(map[string]map[int32]KError, numTopics) + //for i := 0; i < numTopics; i++ { + // name, err := pd.getString() + // if err != nil { + // return err + // } + // + // numErrors, err := pd.getArrayLength() + // if err != nil { + // return err + // } + // + // r.Errors[name] = make(map[int32]KError, numErrors) + // + // for j := 0; j < numErrors; j++ { + // id, err := pd.getInt32() + // if err != nil { + // return err + // } + // + // tmp, err := pd.getInt16() + // if err != nil { + // return err + // } + // r.Errors[name][id] = KError(tmp) + // } + //} + + return nil +} + +func (r *AlterPartitionReassignmentsResponse) key() int16 { + return 45 +} + +func (r *AlterPartitionReassignmentsResponse) version() int16 { + return r.Version +} + +func (r *AlterPartitionReassignmentsResponse) requiredVersion() KafkaVersion { + return MinVersion +} diff --git a/broker.go b/broker.go index 9ca41c91e..86de2e11c 100644 --- a/broker.go +++ b/broker.go @@ -513,6 +513,19 @@ func (b *Broker) CreatePartitions(request *CreatePartitionsRequest) (*CreatePart return response, nil } +//AlterPartitionReassignments sends a alter partition reassignments request and +//returns alter partition reassignments response +func (b *Broker) AlterPartitionReassignments(request *AlterPartitionReassignmentsRequest) (*AlterPartitionReassignmentsResponse, error) { + response := new(AlterPartitionReassignmentsResponse) + + err := b.sendAndReceive(request, response) + if err != nil { + return nil, err + } + + return response, nil +} + //DeleteRecords send a request to delete records and return delete record //response or error func (b *Broker) DeleteRecords(request *DeleteRecordsRequest) (*DeleteRecordsResponse, error) { diff --git a/packet_encoder.go b/packet_encoder.go index 67b8daed8..ba6d0fe88 100644 --- a/packet_encoder.go +++ b/packet_encoder.go @@ -19,6 +19,7 @@ type packetEncoder interface { putBytes(in []byte) error putVarintBytes(in []byte) error putRawBytes(in []byte) error + putCompactString(in string) error putString(in string) error putNullableString(in *string) error putStringArray(in []string) error diff --git a/prep_encoder.go b/prep_encoder.go index b633cd151..ef8561d9e 100644 --- a/prep_encoder.go +++ b/prep_encoder.go @@ -67,6 +67,11 @@ func (pe *prepEncoder) putVarintBytes(in []byte) error { return pe.putRawBytes(in) } +func (pe *prepEncoder) putCompactString(in string) error { + pe.putVarint(int64(len(in) + 1)) + return pe.putRawBytes([]byte(in)) +} + func (pe *prepEncoder) putRawBytes(in []byte) error { if len(in) > math.MaxInt32 { return PacketEncodingError{fmt.Sprintf("byteslice too long (%d)", len(in))} diff --git a/real_encoder.go b/real_encoder.go index 3c75387f7..802f02889 100644 --- a/real_encoder.go +++ b/real_encoder.go @@ -78,6 +78,11 @@ func (re *realEncoder) putVarintBytes(in []byte) error { return re.putRawBytes(in) } +func (re *realEncoder) putCompactString(in string) error { + re.putVarint(int64(len(in) + 1)) + return re.putRawBytes([]byte(in)) +} + func (re *realEncoder) putString(in string) error { re.putInt16(int16(len(in))) copy(re.raw[re.off:], in) diff --git a/request.go b/request.go index 6e4ad8731..d0886d4ee 100644 --- a/request.go +++ b/request.go @@ -166,6 +166,8 @@ func allocateBody(key, version int16) protocolBody { return &CreatePartitionsRequest{} case 42: return &DeleteGroupsRequest{} + case 45: + return &AlterPartitionReassignmentsRequest{} } return nil } From f9334fc8c7ff3c98194b86239f8b66ccfe7784a8 Mon Sep 17 00:00:00 2001 From: Dirk Wilden Date: Wed, 12 Feb 2020 16:28:03 +0100 Subject: [PATCH 02/20] add headerVersion for all requests --- acl_create_request.go | 4 +++ acl_create_response.go | 4 +++ acl_delete_request.go | 4 +++ acl_delete_response.go | 4 +++ acl_describe_request.go | 4 +++ acl_describe_response.go | 4 +++ add_offsets_to_txn_request.go | 4 +++ add_offsets_to_txn_response.go | 4 +++ add_partitions_to_txn_request.go | 4 +++ add_partitions_to_txn_response.go | 4 +++ admin.go | 5 +--- alter_configs_request.go | 4 +++ alter_configs_response.go | 4 +++ alter_partition_reassignments_request.go | 34 +++++++++++++++-------- alter_partition_reassignments_response.go | 4 +++ api_versions_request.go | 4 +++ api_versions_response.go | 4 +++ consumer_metadata_request.go | 4 +++ consumer_metadata_response.go | 4 +++ create_partitions_request.go | 4 +++ create_partitions_response.go | 4 +++ create_topics_request.go | 4 +++ create_topics_response.go | 4 +++ delete_groups_request.go | 4 +++ delete_groups_response.go | 4 +++ delete_records_request.go | 4 +++ delete_records_response.go | 4 +++ delete_topics_request.go | 4 +++ delete_topics_response.go | 4 +++ describe_configs_request.go | 4 +++ describe_configs_response.go | 4 +++ describe_groups_request.go | 4 +++ describe_groups_response.go | 4 +++ describe_log_dirs_request.go | 4 +++ describe_log_dirs_response.go | 4 +++ end_txn_request.go | 4 +++ end_txn_response.go | 4 +++ fetch_request.go | 4 +++ fetch_response.go | 4 +++ find_coordinator_request.go | 4 +++ find_coordinator_response.go | 4 +++ heartbeat_request.go | 4 +++ heartbeat_response.go | 4 +++ init_producer_id_request.go | 4 +++ init_producer_id_response.go | 4 +++ join_group_request.go | 4 +++ join_group_response.go | 4 +++ leave_group_request.go | 4 +++ leave_group_response.go | 4 +++ list_groups_request.go | 4 +++ list_groups_response.go | 4 +++ metadata_request.go | 4 +++ metadata_response.go | 4 +++ offset_commit_request.go | 4 +++ offset_commit_response.go | 4 +++ offset_fetch_request.go | 4 +++ offset_fetch_response.go | 4 +++ offset_request.go | 4 +++ offset_response.go | 4 +++ packet_encoder.go | 3 ++ prep_encoder.go | 17 +++++++++++- produce_request.go | 4 +++ real_encoder.go | 18 +++++++++++- request.go | 16 ++++++++--- sasl_authenticate_request.go | 4 +++ sasl_authenticate_response.go | 4 +++ sasl_handshake_request.go | 4 +++ sasl_handshake_response.go | 4 +++ sync_group_request.go | 4 +++ sync_group_response.go | 4 +++ txn_offset_commit_request.go | 4 +++ txn_offset_commit_response.go | 4 +++ 72 files changed, 335 insertions(+), 22 deletions(-) diff --git a/acl_create_request.go b/acl_create_request.go index da1cdefc3..6d8a70e1a 100644 --- a/acl_create_request.go +++ b/acl_create_request.go @@ -47,6 +47,10 @@ func (c *CreateAclsRequest) version() int16 { return c.Version } +func (c *CreateAclsRequest) headerVersion() int16 { + return 1 +} + func (c *CreateAclsRequest) requiredVersion() KafkaVersion { switch c.Version { case 1: diff --git a/acl_create_response.go b/acl_create_response.go index f5a5e9a64..499616781 100644 --- a/acl_create_response.go +++ b/acl_create_response.go @@ -55,6 +55,10 @@ func (c *CreateAclsResponse) version() int16 { return 0 } +func (c *CreateAclsResponse) headerVersion() int16 { + return 1 +} + func (c *CreateAclsResponse) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/acl_delete_request.go b/acl_delete_request.go index 15908eac9..415252259 100644 --- a/acl_delete_request.go +++ b/acl_delete_request.go @@ -48,6 +48,10 @@ func (d *DeleteAclsRequest) version() int16 { return int16(d.Version) } +func (c *DeleteAclsRequest) headerVersion() int16 { + return 1 +} + func (d *DeleteAclsRequest) requiredVersion() KafkaVersion { switch d.Version { case 1: diff --git a/acl_delete_response.go b/acl_delete_response.go index 3754faeba..f74f23761 100644 --- a/acl_delete_response.go +++ b/acl_delete_response.go @@ -56,6 +56,10 @@ func (d *DeleteAclsResponse) version() int16 { return d.Version } +func (d *DeleteAclsResponse) headerVersion() int16 { + return 1 +} + func (d *DeleteAclsResponse) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/acl_describe_request.go b/acl_describe_request.go index 5222d46ee..29841a5ce 100644 --- a/acl_describe_request.go +++ b/acl_describe_request.go @@ -25,6 +25,10 @@ func (d *DescribeAclsRequest) version() int16 { return int16(d.Version) } +func (d *DescribeAclsRequest) headerVersion() int16 { + return 1 +} + func (d *DescribeAclsRequest) requiredVersion() KafkaVersion { switch d.Version { case 1: diff --git a/acl_describe_response.go b/acl_describe_response.go index a60d39f35..7dadc2b46 100644 --- a/acl_describe_response.go +++ b/acl_describe_response.go @@ -77,6 +77,10 @@ func (d *DescribeAclsResponse) version() int16 { return d.Version } +func (d *DescribeAclsResponse) headerVersion() int16 { + return 1 +} + func (d *DescribeAclsResponse) requiredVersion() KafkaVersion { switch d.Version { case 1: diff --git a/add_offsets_to_txn_request.go b/add_offsets_to_txn_request.go index fc227ab86..95586f9a1 100644 --- a/add_offsets_to_txn_request.go +++ b/add_offsets_to_txn_request.go @@ -48,6 +48,10 @@ func (a *AddOffsetsToTxnRequest) version() int16 { return 0 } +func (a *AddOffsetsToTxnRequest) headerVersion() int16 { + return 1 +} + func (a *AddOffsetsToTxnRequest) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/add_offsets_to_txn_response.go b/add_offsets_to_txn_response.go index c88c1f89f..cf49ba642 100644 --- a/add_offsets_to_txn_response.go +++ b/add_offsets_to_txn_response.go @@ -40,6 +40,10 @@ func (a *AddOffsetsToTxnResponse) version() int16 { return 0 } +func (a *AddOffsetsToTxnResponse) headerVersion() int16 { + return 1 +} + func (a *AddOffsetsToTxnResponse) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/add_partitions_to_txn_request.go b/add_partitions_to_txn_request.go index 8d4b42e34..6289f4514 100644 --- a/add_partitions_to_txn_request.go +++ b/add_partitions_to_txn_request.go @@ -72,6 +72,10 @@ func (a *AddPartitionsToTxnRequest) version() int16 { return 0 } +func (a *AddPartitionsToTxnRequest) headerVersion() int16 { + return 1 +} + func (a *AddPartitionsToTxnRequest) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/add_partitions_to_txn_response.go b/add_partitions_to_txn_response.go index eb4f23eca..4dc2d7551 100644 --- a/add_partitions_to_txn_response.go +++ b/add_partitions_to_txn_response.go @@ -79,6 +79,10 @@ func (a *AddPartitionsToTxnResponse) version() int16 { return 0 } +func (a *AddPartitionsToTxnResponse) headerVersion() int16 { + return 1 +} + func (a *AddPartitionsToTxnResponse) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/admin.go b/admin.go index 40e805608..b5dd9bb51 100644 --- a/admin.go +++ b/admin.go @@ -461,10 +461,7 @@ func (ca *clusterAdmin) AlterPartitionReassignments(topic string, assignment [][ } for i := 0; i < len(assignment); i++ { - for j := 0; j < len(assignment[i]); j++ { - // TODO - request.AddBlock(topic, int32(i), assignment[i][j]) - } + request.AddBlock(topic, int32(i), assignment[i]) } return ca.retryOnError(isErrNoController, func() error { diff --git a/alter_configs_request.go b/alter_configs_request.go index 26c275b83..c88bb604a 100644 --- a/alter_configs_request.go +++ b/alter_configs_request.go @@ -117,6 +117,10 @@ func (a *AlterConfigsRequest) version() int16 { return 0 } +func (a *AlterConfigsRequest) headerVersion() int16 { + return 1 +} + func (a *AlterConfigsRequest) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/alter_configs_response.go b/alter_configs_response.go index 3893663cf..7fa5c5db0 100644 --- a/alter_configs_response.go +++ b/alter_configs_response.go @@ -92,6 +92,10 @@ func (a *AlterConfigsResponse) version() int16 { return 0 } +func (a *AlterConfigsResponse) headerVersion() int16 { + return 1 +} + func (a *AlterConfigsResponse) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/alter_partition_reassignments_request.go b/alter_partition_reassignments_request.go index 2392885ce..c9cb5e559 100644 --- a/alter_partition_reassignments_request.go +++ b/alter_partition_reassignments_request.go @@ -1,17 +1,19 @@ package sarama +import "errors" + type alterPartitionReassignmentsBlock struct { - replicas int32 + replicas []int32 } func (b *alterPartitionReassignmentsBlock) encode(pe packetEncoder, version int16) error { - pe.putInt32(b.replicas) + pe.putCompactInt32Array(b.replicas) return nil } func (b *alterPartitionReassignmentsBlock) decode(pd packetDecoder, version int16) (err error) { - b.replicas, err = pd.getInt32() - return err + //b.replicas, err = pd.getInt32() + return errors.New("bla") } type AlterPartitionReassignmentsRequest struct { @@ -24,24 +26,28 @@ func (r *AlterPartitionReassignmentsRequest) encode(pe packetEncoder) error { pe.putInt32(r.TimeoutMs) - if err := pe.putArrayLength(len(r.blocks)); err != nil { - return err - } + pe.putUVarIntArrayLength(len(r.blocks)) for topic, partitions := range r.blocks { if err := pe.putCompactString(topic); err != nil { return err } - if err := pe.putArrayLength(len(partitions)); err != nil { - return err - } + pe.putUVarIntArrayLength(len(partitions)) for partition, block := range partitions { pe.putInt32(partition) if err := block.encode(pe, r.Version); err != nil { return err } + //another tagged field + pe.putInt8(0); } + //another tagged field + pe.putInt8(0); } + + //another tagged field + pe.putInt8(0); + return nil } @@ -94,11 +100,15 @@ func (r *AlterPartitionReassignmentsRequest) version() int16 { return r.Version } +func (r *AlterPartitionReassignmentsRequest) headerVersion() int16 { + return 2 +} + func (r *AlterPartitionReassignmentsRequest) requiredVersion() KafkaVersion { - return MinVersion + return V2_4_0_0 } -func (r *AlterPartitionReassignmentsRequest) AddBlock(topic string, partitionID int32, replicas int32) { +func (r *AlterPartitionReassignmentsRequest) AddBlock(topic string, partitionID int32, replicas []int32) { if r.blocks == nil { r.blocks = make(map[string]map[int32]*alterPartitionReassignmentsBlock) } diff --git a/alter_partition_reassignments_response.go b/alter_partition_reassignments_response.go index b3aef4694..a315a19fc 100644 --- a/alter_partition_reassignments_response.go +++ b/alter_partition_reassignments_response.go @@ -130,6 +130,10 @@ func (r *AlterPartitionReassignmentsResponse) version() int16 { return r.Version } +func (r *AlterPartitionReassignmentsResponse) headerVersion() int16 { + return 1 +} + func (r *AlterPartitionReassignmentsResponse) requiredVersion() KafkaVersion { return MinVersion } diff --git a/api_versions_request.go b/api_versions_request.go index b33167c0b..d67c5e1e5 100644 --- a/api_versions_request.go +++ b/api_versions_request.go @@ -20,6 +20,10 @@ func (a *ApiVersionsRequest) version() int16 { return 0 } +func (a *ApiVersionsRequest) headerVersion() int16 { + return 1 +} + func (a *ApiVersionsRequest) requiredVersion() KafkaVersion { return V0_10_0_0 } diff --git a/api_versions_response.go b/api_versions_response.go index bb1f0b31a..66bd66c77 100644 --- a/api_versions_response.go +++ b/api_versions_response.go @@ -84,6 +84,10 @@ func (r *ApiVersionsResponse) version() int16 { return 0 } +func (a *ApiVersionsResponse) headerVersion() int16 { + return 1 +} + func (r *ApiVersionsResponse) requiredVersion() KafkaVersion { return V0_10_0_0 } diff --git a/consumer_metadata_request.go b/consumer_metadata_request.go index a8dcaefe8..e5ebdaef5 100644 --- a/consumer_metadata_request.go +++ b/consumer_metadata_request.go @@ -29,6 +29,10 @@ func (r *ConsumerMetadataRequest) version() int16 { return 0 } +func (r *ConsumerMetadataRequest) headerVersion() int16 { + return 1 +} + func (r *ConsumerMetadataRequest) requiredVersion() KafkaVersion { return V0_8_2_0 } diff --git a/consumer_metadata_response.go b/consumer_metadata_response.go index f39a8711c..0e08549ed 100644 --- a/consumer_metadata_response.go +++ b/consumer_metadata_response.go @@ -73,6 +73,10 @@ func (r *ConsumerMetadataResponse) version() int16 { return 0 } +func (r *ConsumerMetadataResponse) headerVersion() int16 { + return 1 +} + func (r *ConsumerMetadataResponse) requiredVersion() KafkaVersion { return V0_8_2_0 } diff --git a/create_partitions_request.go b/create_partitions_request.go index af321e994..46fb04402 100644 --- a/create_partitions_request.go +++ b/create_partitions_request.go @@ -67,6 +67,10 @@ func (r *CreatePartitionsRequest) version() int16 { return 0 } +func (r *CreatePartitionsRequest) headerVersion() int16 { + return 1 +} + func (r *CreatePartitionsRequest) requiredVersion() KafkaVersion { return V1_0_0_0 } diff --git a/create_partitions_response.go b/create_partitions_response.go index bb18204a7..f15a2433c 100644 --- a/create_partitions_response.go +++ b/create_partitions_response.go @@ -63,6 +63,10 @@ func (r *CreatePartitionsResponse) version() int16 { return 0 } +func (r *CreatePartitionsResponse) headerVersion() int16 { + return 1 +} + func (r *CreatePartitionsResponse) requiredVersion() KafkaVersion { return V1_0_0_0 } diff --git a/create_topics_request.go b/create_topics_request.go index 709c0a44e..287acd069 100644 --- a/create_topics_request.go +++ b/create_topics_request.go @@ -79,6 +79,10 @@ func (c *CreateTopicsRequest) version() int16 { return c.Version } +func (r *CreateTopicsRequest) headerVersion() int16 { + return 1 +} + func (c *CreateTopicsRequest) requiredVersion() KafkaVersion { switch c.Version { case 2: diff --git a/create_topics_response.go b/create_topics_response.go index a493e02ac..a600fc419 100644 --- a/create_topics_response.go +++ b/create_topics_response.go @@ -70,6 +70,10 @@ func (c *CreateTopicsResponse) version() int16 { return c.Version } +func (c *CreateTopicsResponse) headerVersion() int16 { + return 1 +} + func (c *CreateTopicsResponse) requiredVersion() KafkaVersion { switch c.Version { case 2: diff --git a/delete_groups_request.go b/delete_groups_request.go index 305a324ac..4ac8bbee4 100644 --- a/delete_groups_request.go +++ b/delete_groups_request.go @@ -21,6 +21,10 @@ func (r *DeleteGroupsRequest) version() int16 { return 0 } +func (r *DeleteGroupsRequest) headerVersion() int16 { + return 1 +} + func (r *DeleteGroupsRequest) requiredVersion() KafkaVersion { return V1_1_0_0 } diff --git a/delete_groups_response.go b/delete_groups_response.go index c067ebb42..99e4da6d7 100644 --- a/delete_groups_response.go +++ b/delete_groups_response.go @@ -65,6 +65,10 @@ func (r *DeleteGroupsResponse) version() int16 { return 0 } +func (r *DeleteGroupsResponse) headerVersion() int16 { + return 1 +} + func (r *DeleteGroupsResponse) requiredVersion() KafkaVersion { return V1_1_0_0 } diff --git a/delete_records_request.go b/delete_records_request.go index 93efafd4d..dc106b17d 100644 --- a/delete_records_request.go +++ b/delete_records_request.go @@ -77,6 +77,10 @@ func (d *DeleteRecordsRequest) version() int16 { return 0 } +func (d *DeleteRecordsRequest) headerVersion() int16 { + return 1 +} + func (d *DeleteRecordsRequest) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/delete_records_response.go b/delete_records_response.go index 733a58b6b..156eb6a17 100644 --- a/delete_records_response.go +++ b/delete_records_response.go @@ -80,6 +80,10 @@ func (d *DeleteRecordsResponse) version() int16 { return 0 } +func (d *DeleteRecordsResponse) headerVersion() int16 { + return 1 +} + func (d *DeleteRecordsResponse) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/delete_topics_request.go b/delete_topics_request.go index 911f67d31..ba6780a8e 100644 --- a/delete_topics_request.go +++ b/delete_topics_request.go @@ -38,6 +38,10 @@ func (d *DeleteTopicsRequest) version() int16 { return d.Version } +func (d *DeleteTopicsRequest) headerVersion() int16 { + return 1 +} + func (d *DeleteTopicsRequest) requiredVersion() KafkaVersion { switch d.Version { case 1: diff --git a/delete_topics_response.go b/delete_topics_response.go index 34225460a..1ce489b10 100644 --- a/delete_topics_response.go +++ b/delete_topics_response.go @@ -68,6 +68,10 @@ func (d *DeleteTopicsResponse) version() int16 { return d.Version } +func (d *DeleteTopicsResponse) headerVersion() int16 { + return 1 +} + func (d *DeleteTopicsResponse) requiredVersion() KafkaVersion { switch d.Version { case 1: diff --git a/describe_configs_request.go b/describe_configs_request.go index ccb587b35..d0c735280 100644 --- a/describe_configs_request.go +++ b/describe_configs_request.go @@ -100,6 +100,10 @@ func (r *DescribeConfigsRequest) version() int16 { return r.Version } +func (r *DescribeConfigsRequest) headerVersion() int16 { + return 1 +} + func (r *DescribeConfigsRequest) requiredVersion() KafkaVersion { switch r.Version { case 1: diff --git a/describe_configs_response.go b/describe_configs_response.go index a18eebab3..844efefb9 100644 --- a/describe_configs_response.go +++ b/describe_configs_response.go @@ -112,6 +112,10 @@ func (r *DescribeConfigsResponse) version() int16 { return r.Version } +func (r *DescribeConfigsResponse) headerVersion() int16 { + return 1 +} + func (r *DescribeConfigsResponse) requiredVersion() KafkaVersion { switch r.Version { case 1: diff --git a/describe_groups_request.go b/describe_groups_request.go index 1fb356777..f8962da58 100644 --- a/describe_groups_request.go +++ b/describe_groups_request.go @@ -21,6 +21,10 @@ func (r *DescribeGroupsRequest) version() int16 { return 0 } +func (r *DescribeGroupsRequest) headerVersion() int16 { + return 1 +} + func (r *DescribeGroupsRequest) requiredVersion() KafkaVersion { return V0_9_0_0 } diff --git a/describe_groups_response.go b/describe_groups_response.go index 542b3a971..349490855 100644 --- a/describe_groups_response.go +++ b/describe_groups_response.go @@ -43,6 +43,10 @@ func (r *DescribeGroupsResponse) version() int16 { return 0 } +func (r *DescribeGroupsResponse) headerVersion() int16 { + return 1 +} + func (r *DescribeGroupsResponse) requiredVersion() KafkaVersion { return V0_9_0_0 } diff --git a/describe_log_dirs_request.go b/describe_log_dirs_request.go index cb1e78152..c0bf04e04 100644 --- a/describe_log_dirs_request.go +++ b/describe_log_dirs_request.go @@ -78,6 +78,10 @@ func (r *DescribeLogDirsRequest) version() int16 { return r.Version } +func (r *DescribeLogDirsRequest) headerVersion() int16 { + return 1 +} + func (r *DescribeLogDirsRequest) requiredVersion() KafkaVersion { return V1_0_0_0 } diff --git a/describe_log_dirs_response.go b/describe_log_dirs_response.go index d207312ef..7691535cf 100644 --- a/describe_log_dirs_response.go +++ b/describe_log_dirs_response.go @@ -61,6 +61,10 @@ func (r *DescribeLogDirsResponse) version() int16 { return r.Version } +func (r *DescribeLogDirsResponse) headerVersion() int16 { + return 1 +} + func (r *DescribeLogDirsResponse) requiredVersion() KafkaVersion { return V1_0_0_0 } diff --git a/end_txn_request.go b/end_txn_request.go index 2cd9b506d..6635425dd 100644 --- a/end_txn_request.go +++ b/end_txn_request.go @@ -45,6 +45,10 @@ func (a *EndTxnRequest) version() int16 { return 0 } +func (r *EndTxnRequest) headerVersion() int16 { + return 1 +} + func (a *EndTxnRequest) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/end_txn_response.go b/end_txn_response.go index 33b27e33d..8d6acf5eb 100644 --- a/end_txn_response.go +++ b/end_txn_response.go @@ -39,6 +39,10 @@ func (e *EndTxnResponse) version() int16 { return 0 } +func (r *EndTxnResponse) headerVersion() int16 { + return 1 +} + func (e *EndTxnResponse) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/fetch_request.go b/fetch_request.go index 9a3e8dd79..f893aeff7 100644 --- a/fetch_request.go +++ b/fetch_request.go @@ -239,6 +239,10 @@ func (r *FetchRequest) version() int16 { return r.Version } +func (r *FetchRequest) headerVersion() int16 { + return 1 +} + func (r *FetchRequest) requiredVersion() KafkaVersion { switch r.Version { case 0: diff --git a/fetch_response.go b/fetch_response.go index dc0aeed2e..eee282c30 100644 --- a/fetch_response.go +++ b/fetch_response.go @@ -335,6 +335,10 @@ func (r *FetchResponse) version() int16 { return r.Version } +func (r *FetchResponse) headerVersion() int16 { + return 1 +} + func (r *FetchResponse) requiredVersion() KafkaVersion { switch r.Version { case 0: diff --git a/find_coordinator_request.go b/find_coordinator_request.go index ff2ad206c..597bcbf78 100644 --- a/find_coordinator_request.go +++ b/find_coordinator_request.go @@ -51,6 +51,10 @@ func (f *FindCoordinatorRequest) version() int16 { return f.Version } +func (r *FindCoordinatorRequest) headerVersion() int16 { + return 1 +} + func (f *FindCoordinatorRequest) requiredVersion() KafkaVersion { switch f.Version { case 1: diff --git a/find_coordinator_response.go b/find_coordinator_response.go index 9c900e8b7..b2971c9b4 100644 --- a/find_coordinator_response.go +++ b/find_coordinator_response.go @@ -82,6 +82,10 @@ func (f *FindCoordinatorResponse) version() int16 { return f.Version } +func (r *FindCoordinatorResponse) headerVersion() int16 { + return 1 +} + func (f *FindCoordinatorResponse) requiredVersion() KafkaVersion { switch f.Version { case 1: diff --git a/heartbeat_request.go b/heartbeat_request.go index ce49c4739..e9d9af191 100644 --- a/heartbeat_request.go +++ b/heartbeat_request.go @@ -42,6 +42,10 @@ func (r *HeartbeatRequest) version() int16 { return 0 } +func (r *HeartbeatRequest) headerVersion() int16 { + return 1 +} + func (r *HeartbeatRequest) requiredVersion() KafkaVersion { return V0_9_0_0 } diff --git a/heartbeat_response.go b/heartbeat_response.go index 766f5fdec..6589a2dfb 100644 --- a/heartbeat_response.go +++ b/heartbeat_response.go @@ -27,6 +27,10 @@ func (r *HeartbeatResponse) version() int16 { return 0 } +func (r *HeartbeatResponse) headerVersion() int16 { + return 1 +} + func (r *HeartbeatResponse) requiredVersion() KafkaVersion { return V0_9_0_0 } diff --git a/init_producer_id_request.go b/init_producer_id_request.go index 8ceb6c232..689444397 100644 --- a/init_producer_id_request.go +++ b/init_producer_id_request.go @@ -38,6 +38,10 @@ func (i *InitProducerIDRequest) version() int16 { return 0 } +func (i *InitProducerIDRequest) headerVersion() int16 { + return 1 +} + func (i *InitProducerIDRequest) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/init_producer_id_response.go b/init_producer_id_response.go index 1b32eb085..5c4aca2c3 100644 --- a/init_producer_id_response.go +++ b/init_producer_id_response.go @@ -50,6 +50,10 @@ func (i *InitProducerIDResponse) version() int16 { return 0 } +func (i *InitProducerIDResponse) headerVersion() int16 { + return 1 +} + func (i *InitProducerIDResponse) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/join_group_request.go b/join_group_request.go index 97e9299ea..3734e82e4 100644 --- a/join_group_request.go +++ b/join_group_request.go @@ -134,6 +134,10 @@ func (r *JoinGroupRequest) version() int16 { return r.Version } +func (r *JoinGroupRequest) headerVersion() int16 { + return 1 +} + func (r *JoinGroupRequest) requiredVersion() KafkaVersion { switch r.Version { case 2: diff --git a/join_group_response.go b/join_group_response.go index 5752acc8a..bebafbf32 100644 --- a/join_group_response.go +++ b/join_group_response.go @@ -123,6 +123,10 @@ func (r *JoinGroupResponse) version() int16 { return r.Version } +func (r *JoinGroupResponse) headerVersion() int16 { + return 1 +} + func (r *JoinGroupResponse) requiredVersion() KafkaVersion { switch r.Version { case 2: diff --git a/leave_group_request.go b/leave_group_request.go index e17742748..d7789b68d 100644 --- a/leave_group_request.go +++ b/leave_group_request.go @@ -35,6 +35,10 @@ func (r *LeaveGroupRequest) version() int16 { return 0 } +func (r *LeaveGroupRequest) headerVersion() int16 { + return 1 +} + func (r *LeaveGroupRequest) requiredVersion() KafkaVersion { return V0_9_0_0 } diff --git a/leave_group_response.go b/leave_group_response.go index d60c626da..4544e2e08 100644 --- a/leave_group_response.go +++ b/leave_group_response.go @@ -27,6 +27,10 @@ func (r *LeaveGroupResponse) version() int16 { return 0 } +func (r *LeaveGroupResponse) headerVersion() int16 { + return 1 +} + func (r *LeaveGroupResponse) requiredVersion() KafkaVersion { return V0_9_0_0 } diff --git a/list_groups_request.go b/list_groups_request.go index 3b16abf7f..ed44cc27e 100644 --- a/list_groups_request.go +++ b/list_groups_request.go @@ -19,6 +19,10 @@ func (r *ListGroupsRequest) version() int16 { return 0 } +func (r *ListGroupsRequest) headerVersion() int16 { + return 1 +} + func (r *ListGroupsRequest) requiredVersion() KafkaVersion { return V0_9_0_0 } diff --git a/list_groups_response.go b/list_groups_response.go index 56115d4c7..7b1c495a1 100644 --- a/list_groups_response.go +++ b/list_groups_response.go @@ -64,6 +64,10 @@ func (r *ListGroupsResponse) version() int16 { return 0 } +func (r *ListGroupsResponse) headerVersion() int16 { + return 1 +} + func (r *ListGroupsResponse) requiredVersion() KafkaVersion { return V0_9_0_0 } diff --git a/metadata_request.go b/metadata_request.go index 1b590d368..e835f5a9c 100644 --- a/metadata_request.go +++ b/metadata_request.go @@ -65,6 +65,10 @@ func (r *MetadataRequest) version() int16 { return r.Version } +func (r *MetadataRequest) headerVersion() int16 { + return 1 +} + func (r *MetadataRequest) requiredVersion() KafkaVersion { switch r.Version { case 1: diff --git a/metadata_response.go b/metadata_response.go index 916992d24..c6b43d7ca 100644 --- a/metadata_response.go +++ b/metadata_response.go @@ -255,6 +255,10 @@ func (r *MetadataResponse) version() int16 { return r.Version } +func (r *MetadataResponse) headerVersion() int16 { + return 1 +} + func (r *MetadataResponse) requiredVersion() KafkaVersion { switch r.Version { case 1: diff --git a/offset_commit_request.go b/offset_commit_request.go index 5732ed95c..9931cade5 100644 --- a/offset_commit_request.go +++ b/offset_commit_request.go @@ -170,6 +170,10 @@ func (r *OffsetCommitRequest) version() int16 { return r.Version } +func (r *OffsetCommitRequest) headerVersion() int16 { + return 1 +} + func (r *OffsetCommitRequest) requiredVersion() KafkaVersion { switch r.Version { case 1: diff --git a/offset_commit_response.go b/offset_commit_response.go index e842298db..8648a49df 100644 --- a/offset_commit_response.go +++ b/offset_commit_response.go @@ -94,6 +94,10 @@ func (r *OffsetCommitResponse) version() int16 { return r.Version } +func (r *OffsetCommitResponse) headerVersion() int16 { + return 1 +} + func (r *OffsetCommitResponse) requiredVersion() KafkaVersion { switch r.Version { case 1: diff --git a/offset_fetch_request.go b/offset_fetch_request.go index 68608241f..51e9faa3f 100644 --- a/offset_fetch_request.go +++ b/offset_fetch_request.go @@ -68,6 +68,10 @@ func (r *OffsetFetchRequest) version() int16 { return r.Version } +func (r *OffsetFetchRequest) headerVersion() int16 { + return 1 +} + func (r *OffsetFetchRequest) requiredVersion() KafkaVersion { switch r.Version { case 1: diff --git a/offset_fetch_response.go b/offset_fetch_response.go index 9e2570280..5632e68cc 100644 --- a/offset_fetch_response.go +++ b/offset_fetch_response.go @@ -155,6 +155,10 @@ func (r *OffsetFetchResponse) version() int16 { return r.Version } +func (r *OffsetFetchResponse) headerVersion() int16 { + return 1 +} + func (r *OffsetFetchResponse) requiredVersion() KafkaVersion { switch r.Version { case 1: diff --git a/offset_request.go b/offset_request.go index 58e223762..c0b3305f6 100644 --- a/offset_request.go +++ b/offset_request.go @@ -116,6 +116,10 @@ func (r *OffsetRequest) version() int16 { return r.Version } +func (r *OffsetRequest) headerVersion() int16 { + return 1 +} + func (r *OffsetRequest) requiredVersion() KafkaVersion { switch r.Version { case 1: diff --git a/offset_response.go b/offset_response.go index 8b2193f9a..0fdfa2281 100644 --- a/offset_response.go +++ b/offset_response.go @@ -150,6 +150,10 @@ func (r *OffsetResponse) version() int16 { return r.Version } +func (r *OffsetResponse) headerVersion() int16 { + return 1 +} + func (r *OffsetResponse) requiredVersion() KafkaVersion { switch r.Version { case 1: diff --git a/packet_encoder.go b/packet_encoder.go index ba6d0fe88..3fe6bd63a 100644 --- a/packet_encoder.go +++ b/packet_encoder.go @@ -12,6 +12,8 @@ type packetEncoder interface { putInt32(in int32) putInt64(in int64) putVarint(in int64) + putUVarint(in uint64) + putUVarIntArrayLength(in int) putArrayLength(in int) error putBool(in bool) @@ -23,6 +25,7 @@ type packetEncoder interface { putString(in string) error putNullableString(in *string) error putStringArray(in []string) error + putCompactInt32Array(in []int32) error putInt32Array(in []int32) error putInt64Array(in []int64) error diff --git a/prep_encoder.go b/prep_encoder.go index ef8561d9e..785af6455 100644 --- a/prep_encoder.go +++ b/prep_encoder.go @@ -36,6 +36,11 @@ func (pe *prepEncoder) putVarint(in int64) { pe.length += binary.PutVarint(buf[:], in) } +func (pe *prepEncoder) putUVarint(in uint64) { + var buf [binary.MaxVarintLen64]byte + pe.length += binary.PutUvarint(buf[:], in) +} + func (pe *prepEncoder) putArrayLength(in int) error { if in > math.MaxInt32 { return PacketEncodingError{fmt.Sprintf("array too long (%d)", in)} @@ -44,6 +49,10 @@ func (pe *prepEncoder) putArrayLength(in int) error { return nil } +func (pe *prepEncoder) putUVarIntArrayLength(in int) { + pe.putUVarint(uint64(in+1)) +} + func (pe *prepEncoder) putBool(in bool) { pe.length++ } @@ -68,7 +77,7 @@ func (pe *prepEncoder) putVarintBytes(in []byte) error { } func (pe *prepEncoder) putCompactString(in string) error { - pe.putVarint(int64(len(in) + 1)) + pe.putUVarint(uint64(len(in) + 1)) return pe.putRawBytes([]byte(in)) } @@ -112,6 +121,12 @@ func (pe *prepEncoder) putStringArray(in []string) error { return nil } +func (pe *prepEncoder) putCompactInt32Array(in []int32) error { + pe.putUVarint(uint64(len(in))+1) + pe.length += 4 * len(in) + return nil +} + func (pe *prepEncoder) putInt32Array(in []int32) error { err := pe.putArrayLength(len(in)) if err != nil { diff --git a/produce_request.go b/produce_request.go index 178972a0f..0034651e2 100644 --- a/produce_request.go +++ b/produce_request.go @@ -206,6 +206,10 @@ func (r *ProduceRequest) version() int16 { return r.Version } +func (r *ProduceRequest) headerVersion() int16 { + return 1 +} + func (r *ProduceRequest) requiredVersion() KafkaVersion { switch r.Version { case 1: diff --git a/real_encoder.go b/real_encoder.go index 802f02889..b037f85e8 100644 --- a/real_encoder.go +++ b/real_encoder.go @@ -39,11 +39,19 @@ func (re *realEncoder) putVarint(in int64) { re.off += binary.PutVarint(re.raw[re.off:], in) } +func (re *realEncoder) putUVarint(in uint64) { + re.off += binary.PutUvarint(re.raw[re.off:], in) +} + func (re *realEncoder) putArrayLength(in int) error { re.putInt32(int32(in)) return nil } +func (re *realEncoder) putUVarIntArrayLength(in int) { + re.putUVarint(uint64(in+1)) +} + func (re *realEncoder) putBool(in bool) { if in { re.putInt8(1) @@ -79,7 +87,7 @@ func (re *realEncoder) putVarintBytes(in []byte) error { } func (re *realEncoder) putCompactString(in string) error { - re.putVarint(int64(len(in) + 1)) + re.putUVarint(uint64(len(in) + 1)) return re.putRawBytes([]byte(in)) } @@ -113,6 +121,14 @@ func (re *realEncoder) putStringArray(in []string) error { return nil } +func (re *realEncoder) putCompactInt32Array(in []int32) error { + re.putUVarint(uint64(len(in))+1) + for _, val := range in { + re.putInt32(val) + } + return nil +} + func (re *realEncoder) putInt32Array(in []int32) error { err := re.putArrayLength(len(in)) if err != nil { diff --git a/request.go b/request.go index d0886d4ee..62b8913ed 100644 --- a/request.go +++ b/request.go @@ -11,6 +11,7 @@ type protocolBody interface { versionedDecoder key() int16 version() int16 + headerVersion() int16 requiredVersion() KafkaVersion } @@ -26,12 +27,19 @@ func (r *request) encode(pe packetEncoder) error { pe.putInt16(r.body.version()) pe.putInt32(r.correlationID) - err := pe.putString(r.clientID) - if err != nil { - return err + if r.body.headerVersion() >= 1 { + err := pe.putString(r.clientID) + if err != nil { + return err + } + } + + if r.body.headerVersion() >= 2 { + // we don't use tag headers at the moment so we just put an array length of 0 + pe.putUVarint(0); } - err = r.body.encode(pe) + err := r.body.encode(pe) if err != nil { return err } diff --git a/sasl_authenticate_request.go b/sasl_authenticate_request.go index 54c8b0992..90504df6f 100644 --- a/sasl_authenticate_request.go +++ b/sasl_authenticate_request.go @@ -24,6 +24,10 @@ func (r *SaslAuthenticateRequest) version() int16 { return 0 } +func (r *SaslAuthenticateRequest) headerVersion() int16 { + return 1 +} + func (r *SaslAuthenticateRequest) requiredVersion() KafkaVersion { return V1_0_0_0 } diff --git a/sasl_authenticate_response.go b/sasl_authenticate_response.go index 0038c3f36..a91442cd4 100644 --- a/sasl_authenticate_response.go +++ b/sasl_authenticate_response.go @@ -39,6 +39,10 @@ func (r *SaslAuthenticateResponse) version() int16 { return 0 } +func (r *SaslAuthenticateResponse) headerVersion() int16 { + return 1 +} + func (r *SaslAuthenticateResponse) requiredVersion() KafkaVersion { return V1_0_0_0 } diff --git a/sasl_handshake_request.go b/sasl_handshake_request.go index fe5ba0504..74dc3072f 100644 --- a/sasl_handshake_request.go +++ b/sasl_handshake_request.go @@ -29,6 +29,10 @@ func (r *SaslHandshakeRequest) version() int16 { return r.Version } +func (r *SaslHandshakeRequest) headerVersion() int16 { + return 1 +} + func (r *SaslHandshakeRequest) requiredVersion() KafkaVersion { return V0_10_0_0 } diff --git a/sasl_handshake_response.go b/sasl_handshake_response.go index ef290d4bc..6b99843bb 100644 --- a/sasl_handshake_response.go +++ b/sasl_handshake_response.go @@ -33,6 +33,10 @@ func (r *SaslHandshakeResponse) version() int16 { return 0 } +func (r *SaslHandshakeResponse) headerVersion() int16 { + return 1 +} + func (r *SaslHandshakeResponse) requiredVersion() KafkaVersion { return V0_10_0_0 } diff --git a/sync_group_request.go b/sync_group_request.go index fe207080e..ac6ecb13e 100644 --- a/sync_group_request.go +++ b/sync_group_request.go @@ -77,6 +77,10 @@ func (r *SyncGroupRequest) version() int16 { return 0 } +func (r *SyncGroupRequest) headerVersion() int16 { + return 1 +} + func (r *SyncGroupRequest) requiredVersion() KafkaVersion { return V0_9_0_0 } diff --git a/sync_group_response.go b/sync_group_response.go index 194b382b4..3a6f8a7bc 100644 --- a/sync_group_response.go +++ b/sync_group_response.go @@ -36,6 +36,10 @@ func (r *SyncGroupResponse) version() int16 { return 0 } +func (r *SyncGroupResponse) headerVersion() int16 { + return 1 +} + func (r *SyncGroupResponse) requiredVersion() KafkaVersion { return V0_9_0_0 } diff --git a/txn_offset_commit_request.go b/txn_offset_commit_request.go index 71e95b814..c4043a335 100644 --- a/txn_offset_commit_request.go +++ b/txn_offset_commit_request.go @@ -91,6 +91,10 @@ func (a *TxnOffsetCommitRequest) version() int16 { return 0 } +func (a *TxnOffsetCommitRequest) headerVersion() int16 { + return 1 +} + func (a *TxnOffsetCommitRequest) requiredVersion() KafkaVersion { return V0_11_0_0 } diff --git a/txn_offset_commit_response.go b/txn_offset_commit_response.go index 6c980f406..25f5475d2 100644 --- a/txn_offset_commit_response.go +++ b/txn_offset_commit_response.go @@ -78,6 +78,10 @@ func (a *TxnOffsetCommitResponse) version() int16 { return 0 } +func (a *TxnOffsetCommitResponse) headerVersion() int16 { + return 1 +} + func (a *TxnOffsetCommitResponse) requiredVersion() KafkaVersion { return V0_11_0_0 } From d926c43c5a2b883cb6486e02d09fc7ed3253c65c Mon Sep 17 00:00:00 2001 From: sladkoff Date: Wed, 12 Feb 2020 19:19:34 +0100 Subject: [PATCH 03/20] Implement AlterPartitionReassignmentsResponse decoder --- admin.go | 7 +- alter_partition_reassignments_response.go | 211 ++++++++++++---------- packet_decoder.go | 4 + real_decoder.go | 59 ++++++ real_encoder.go | 4 +- 5 files changed, 183 insertions(+), 102 deletions(-) diff --git a/admin.go b/admin.go index b5dd9bb51..298a866a6 100644 --- a/admin.go +++ b/admin.go @@ -472,13 +472,14 @@ func (ca *clusterAdmin) AlterPartitionReassignments(topic string, assignment [][ rsp, err := b.AlterPartitionReassignments(request) - _ = rsp - if err != nil { return err } - // TODO error handling + if rsp.ErrorCode > 0 || len(rsp.Errors) > 0 { + // TODO pretty error from response + return errors.New("Kafka server returned errors during replication-factor assignment") + } return nil }) diff --git a/alter_partition_reassignments_response.go b/alter_partition_reassignments_response.go index a315a19fc..3b91a9052 100644 --- a/alter_partition_reassignments_response.go +++ b/alter_partition_reassignments_response.go @@ -1,68 +1,70 @@ package sarama -//type alterPartitionReassignmentsErrorBlock struct { -// errorCode KError -// errorMessage string -//} -// -//func (b *alterPartitionReassignmentsErrorBlock) encode(pe packetEncoder) error { -// pe.putInt32(b.errorCode) -// pe.putString(b.errorMessage) -// -// return nil -//} -// -//func (b *alterPartitionReassignmentsErrorBlock) decode(pd packetDecoder) (err error) { -// if b.errorCode, err = pd.getInt32(); err != nil { -// return err -// } -// b.errorMessage, err = pd.getString() -// return err -//} +type alterPartitionReassignmentsErrorBlock struct { + errorCode KError + errorMessage string +} + +func (b *alterPartitionReassignmentsErrorBlock) encode(pe packetEncoder) error { + pe.putInt32(int32(b.errorCode)) + pe.putString(b.errorMessage) + + return nil +} + +func (b *alterPartitionReassignmentsErrorBlock) decode(pd packetDecoder) (err error) { + errorCode, err := pd.getInt32() + if err != nil { + return err + } + b.errorCode = KError(errorCode) + b.errorMessage, err = pd.getString() + return err +} type AlterPartitionReassignmentsResponse struct { Version int16 ThrottleTimeMs int32 ErrorCode KError ErrorMessage *string - //Errors map[string]map[int32]*alterPartitionReassignmentsErrorBlock + Errors map[string]map[int32]*alterPartitionReassignmentsErrorBlock } -//func (r *AlterPartitionReassignmentsResponse) AddError(topic string, partition int32, kerror KError) { -// // TODO -// if r.Errors == nil { -// r.Errors = make(map[string]map[int32]KError) -// } -// partitions := r.Errors[topic] -// if partitions == nil { -// partitions = make(map[int32]KError) -// r.Errors[topic] = partitions -// } -// partitions[partition] = kerror -//} +func (r *AlterPartitionReassignmentsResponse) AddError(topic string, partition int32, kerror KError, message *string) { + if r.Errors == nil { + r.Errors = make(map[string]map[int32]*alterPartitionReassignmentsErrorBlock) + } + partitions := r.Errors[topic] + if partitions == nil { + partitions = make(map[int32]*alterPartitionReassignmentsErrorBlock) + r.Errors[topic] = partitions + } + + partitions[partition] = &alterPartitionReassignmentsErrorBlock{errorCode: kerror, errorMessage: *message} +} func (r *AlterPartitionReassignmentsResponse) encode(pe packetEncoder) error { pe.putInt32(r.ThrottleTimeMs) - //pe.putInt16(int16(r.ErrorCode)) - //if err := pe.putNullableString(r.ErrorMessage); err != nil { - // return err - //} - - //if err := pe.putArrayLength(len(r.Errors)); err != nil { - // return err - //} - //for topic, partitions := range r.Errors { - // if err := pe.putString(topic); err != nil { - // return err - // } - // if err := pe.putArrayLength(len(partitions)); err != nil { - // return err - // } - // for partition, kerror := range partitions { - // pe.putInt32(partition) - // pe.putInt16(int16(kerror)) - // } - //} + pe.putInt16(int16(r.ErrorCode)) + if err := pe.putNullableString(r.ErrorMessage); err != nil { + return err + } + + if err := pe.putArrayLength(len(r.Errors)); err != nil { + return err + } + for topic, partitions := range r.Errors { + if err := pe.putString(topic); err != nil { + return err + } + if err := pe.putArrayLength(len(partitions)); err != nil { + return err + } + for partition, kerror := range partitions { + pe.putInt32(partition) + pe.putInt16(int16(kerror.errorCode)) + } + } return nil } @@ -73,51 +75,66 @@ func (r *AlterPartitionReassignmentsResponse) decode(pd packetDecoder, version i return err } - //kerr, err := pd.getInt16(); - //if err != nil { - // return err - //} - // - //r.ErrorCode = KError(kerr) - // - //if r.ErrorMessage, err = pd.getNullableString(); err != nil { - // return err - //} + kerr, err := pd.getInt16() + if err != nil { + return err + } - return nil + r.ErrorCode = KError(kerr) - //numTopics, err := pd.getArrayLength() - //if err != nil || numTopics == 0 { - // return err - //} - // - //r.Errors = make(map[string]map[int32]KError, numTopics) - //for i := 0; i < numTopics; i++ { - // name, err := pd.getString() - // if err != nil { - // return err - // } - // - // numErrors, err := pd.getArrayLength() - // if err != nil { - // return err - // } - // - // r.Errors[name] = make(map[int32]KError, numErrors) - // - // for j := 0; j < numErrors; j++ { - // id, err := pd.getInt32() - // if err != nil { - // return err - // } - // - // tmp, err := pd.getInt16() - // if err != nil { - // return err - // } - // r.Errors[name][id] = KError(tmp) - // } - //} + if r.ErrorMessage, err = pd.getCompactNullableString(); err != nil { + return err + } + + numTopics, err := pd.getCompactArrayLength() + if err != nil || numTopics == 0 { + return err + } + + r.Errors = make(map[string]map[int32]*alterPartitionReassignmentsErrorBlock, numTopics) + for i := 0; i < int(numTopics); i++ { + name, err := pd.getCompactString() + if err != nil { + return err + } + + ongoingPartitionReassignments, err := pd.getCompactArrayLength() + if err != nil { + return err + } + + r.Errors[name] = make(map[int32]*alterPartitionReassignmentsErrorBlock, ongoingPartitionReassignments) + + for j := 0; j < ongoingPartitionReassignments; j++ { + partition, err := pd.getInt32() + if err != nil { + return err + } + errorCode, err := pd.getInt16() + if err != nil { + return err + } + errorMessage, err := pd.getCompactNullableString() + if err != nil { + return err + } + + if errorCode != 0 { + if errorMessage == nil { + errorMessage = new(string) + } + + r.AddError(name, partition, KError(errorCode), errorMessage) + } + + // Tag buffer? + pd.getInt8() + } + // Tag buffer? + pd.getInt8() + } + // Tag buffer? + pd.getInt8() return nil } @@ -131,9 +148,9 @@ func (r *AlterPartitionReassignmentsResponse) version() int16 { } func (r *AlterPartitionReassignmentsResponse) headerVersion() int16 { - return 1 + return 2 } func (r *AlterPartitionReassignmentsResponse) requiredVersion() KafkaVersion { - return MinVersion + return V2_4_0_0 } diff --git a/packet_decoder.go b/packet_decoder.go index 9be854c07..bd95a3b02 100644 --- a/packet_decoder.go +++ b/packet_decoder.go @@ -10,7 +10,9 @@ type packetDecoder interface { getInt32() (int32, error) getInt64() (int64, error) getVarint() (int64, error) + getUVarint() (uint64, error) getArrayLength() (int, error) + getCompactArrayLength() (int, error) getBool() (bool, error) // Collections @@ -19,6 +21,8 @@ type packetDecoder interface { getRawBytes(length int) ([]byte, error) getString() (string, error) getNullableString() (*string, error) + getCompactString() (string, error) + getCompactNullableString() (*string, error) getInt32Array() ([]int32, error) getInt64Array() ([]int64, error) getStringArray() ([]string, error) diff --git a/real_decoder.go b/real_decoder.go index 6c5a1b9b0..58632ccc4 100644 --- a/real_decoder.go +++ b/real_decoder.go @@ -73,6 +73,20 @@ func (rd *realDecoder) getVarint() (int64, error) { return tmp, nil } +func (rd *realDecoder) getUVarint() (uint64, error) { + tmp, n := binary.Uvarint(rd.raw[rd.off:]) + if n == 0 { + rd.off = int(len(rd.raw)) + return 0, ErrInsufficientData + } + if n < 0 { + rd.off -= n + return 0, errVarintOverflow + } + rd.off += n + return tmp, nil +} + func (rd *realDecoder) getArrayLength() (int, error) { if rd.remaining() < 4 { rd.off = len(rd.raw) @@ -89,6 +103,19 @@ func (rd *realDecoder) getArrayLength() (int, error) { return tmp, nil } +func (rd *realDecoder) getCompactArrayLength() (int, error) { + n, err := rd.getUVarint() + if err != nil { + return 0, err + } + + if n == 0 { + return 0, nil + } + + return int(n) - 1, nil +} + func (rd *realDecoder) getBool() (bool, error) { b, err := rd.getInt8() if err != nil || b == 0 { @@ -167,6 +194,38 @@ func (rd *realDecoder) getNullableString() (*string, error) { return &tmpStr, err } +func (rd *realDecoder) getCompactString() (string, error) { + n, err := rd.getUVarint() + if err != nil { + return "", err + } + + var length = int(n - 1) + + tmpStr := string(rd.raw[rd.off : rd.off+length]) + rd.off += length + return tmpStr, nil +} + +func (rd *realDecoder) getCompactNullableString() (*string, error) { + n, err := rd.getUVarint() + + if err != nil { + return nil, err + } + + var length = int(n - 1) + + if length < 0 { + rd.off += 1 + return nil, err + } + + tmpStr := string(rd.raw[rd.off : rd.off+length]) + rd.off += length + return &tmpStr, err +} + func (rd *realDecoder) getInt32Array() ([]int32, error) { if rd.remaining() < 4 { rd.off = len(rd.raw) diff --git a/real_encoder.go b/real_encoder.go index b037f85e8..4ba962258 100644 --- a/real_encoder.go +++ b/real_encoder.go @@ -49,7 +49,7 @@ func (re *realEncoder) putArrayLength(in int) error { } func (re *realEncoder) putUVarIntArrayLength(in int) { - re.putUVarint(uint64(in+1)) + re.putUVarint(uint64(in + 1)) } func (re *realEncoder) putBool(in bool) { @@ -122,7 +122,7 @@ func (re *realEncoder) putStringArray(in []string) error { } func (re *realEncoder) putCompactInt32Array(in []int32) error { - re.putUVarint(uint64(len(in))+1) + re.putUVarint(uint64(len(in)) + 1) for _, val := range in { re.putInt32(val) } From 24f957ed1dfca700e8269e602b6702e602ac922b Mon Sep 17 00:00:00 2001 From: Dirk Wilden Date: Thu, 13 Feb 2020 08:59:21 +0100 Subject: [PATCH 04/20] add tests for alter_partition_reassignments --- alter_partition_reassignments_request.go | 89 +++++++++++------- alter_partition_reassignments_request_test.go | 91 +++++++++++++++++++ alter_partition_reassignments_response.go | 60 +++++++----- ...r_partition_reassignments_response_test.go | 24 +++++ packet_encoder.go | 3 +- prep_encoder.go | 17 +++- real_encoder.go | 14 ++- request.go | 10 +- request_test.go | 13 ++- 9 files changed, 260 insertions(+), 61 deletions(-) create mode 100644 alter_partition_reassignments_request_test.go create mode 100644 alter_partition_reassignments_response_test.go diff --git a/alter_partition_reassignments_request.go b/alter_partition_reassignments_request.go index c9cb5e559..fbd7b7c0f 100644 --- a/alter_partition_reassignments_request.go +++ b/alter_partition_reassignments_request.go @@ -1,19 +1,33 @@ package sarama -import "errors" - type alterPartitionReassignmentsBlock struct { replicas []int32 } func (b *alterPartitionReassignmentsBlock) encode(pe packetEncoder, version int16) error { pe.putCompactInt32Array(b.replicas) + // tagged field + pe.putInt8(0) return nil } func (b *alterPartitionReassignmentsBlock) decode(pd packetDecoder, version int16) (err error) { - //b.replicas, err = pd.getInt32() - return errors.New("bla") + + replicaCount, err := pd.getCompactArrayLength() + if err != nil { + return err + } + + b.replicas = make([]int32, replicaCount) + + for i := 0; i < replicaCount; i++ { + if replica, err := pd.getInt32(); err != nil { + return err + } else { + b.replicas[i] = replica + } + } + return nil } type AlterPartitionReassignmentsRequest struct { @@ -23,30 +37,27 @@ type AlterPartitionReassignmentsRequest struct { } func (r *AlterPartitionReassignmentsRequest) encode(pe packetEncoder) error { - pe.putInt32(r.TimeoutMs) - pe.putUVarIntArrayLength(len(r.blocks)) + pe.putCompactArrayLength(len(r.blocks)) for topic, partitions := range r.blocks { if err := pe.putCompactString(topic); err != nil { return err } - pe.putUVarIntArrayLength(len(partitions)) + pe.putCompactArrayLength(len(partitions)) for partition, block := range partitions { pe.putInt32(partition) if err := block.encode(pe, r.Version); err != nil { return err } - //another tagged field - pe.putInt8(0); } //another tagged field - pe.putInt8(0); + pe.putInt8(0) } //another tagged field - pe.putInt8(0); + pe.putInt8(0) return nil } @@ -58,37 +69,53 @@ func (r *AlterPartitionReassignmentsRequest) decode(pd packetDecoder, version in return err } - topicCount, err := pd.getArrayLength() + topicCount, err := pd.getCompactArrayLength() if err != nil { return err } - if topicCount == 0 { - return nil - } - r.blocks = make(map[string]map[int32]*alterPartitionReassignmentsBlock) - for i := 0; i < topicCount; i++ { - topic, err := pd.getString() - if err != nil { - return err - } - partitionCount, err := pd.getArrayLength() - if err != nil { - return err - } - r.blocks[topic] = make(map[int32]*alterPartitionReassignmentsBlock) - for j := 0; j < partitionCount; j++ { - partition, err := pd.getInt32() + if topicCount > 0 { + r.blocks = make(map[string]map[int32]*alterPartitionReassignmentsBlock) + for i := 0; i < topicCount; i++ { + topic, err := pd.getCompactString() + if err != nil { + return err + } + partitionCount, err := pd.getCompactArrayLength() if err != nil { return err } - block := &alterPartitionReassignmentsBlock{} - if err := block.decode(pd, r.Version); err != nil { + r.blocks[topic] = make(map[int32]*alterPartitionReassignmentsBlock) + for j := 0; j < partitionCount; j++ { + partition, err := pd.getInt32() + if err != nil { + return err + } + block := &alterPartitionReassignmentsBlock{} + if err := block.decode(pd, r.Version); err != nil { + return err + } + r.blocks[topic][partition] = block + + // empty tagged fields array + _, err = pd.getUVarint() + if err != nil { + return err + } + } + // empty tagged fields array + _, err = pd.getUVarint() + if err != nil { return err } - r.blocks[topic][partition] = block } } + // empty tagged fields array + _, err = pd.getUVarint() + if err != nil { + return err + } + return } diff --git a/alter_partition_reassignments_request_test.go b/alter_partition_reassignments_request_test.go new file mode 100644 index 000000000..f6e125e6a --- /dev/null +++ b/alter_partition_reassignments_request_test.go @@ -0,0 +1,91 @@ +package sarama + +import "testing" + +var ( + alterPartitionReassignmentsRequestNoBlock = []byte{ + 0, 0, 39, 16, // timout 10000 + 1, // 1-1=0 blocks + 0, // empty tagged fields + } + + alterPartitionReassignmentsRequestOneBlock = []byte{ + 0, 0, 39, 16, // timout 10000 + 2, // 2-1=1 block + 6, 116, 111, 112, 105, 99, // topic name "topic" as compact string + 2, // 2-1=1 partitions + 0, 0, 0, 0, // partitionId + 3, // 3-1=2 replica array size + 0, 0, 3, 232, // replica 1000 + 0, 0, 3, 233, // replica 1001 + 0, 0, 0, // empty tagged fields + } + + alterPartitionReassignmentsRequestTwoBlocks = []byte{ + 0, 0, 39, 16, // timout 10000 + 3, // 3-1=2 blocks + 6, 116, 111, 112, 105, 99, // topic name "topic" as compact string + 2, // 2-1=1 partitions + 0, 0, 0, 0, // partitionId + 3, // 3-1=2 replica array size + 0, 0, 3, 232, // replica 1000 + 0, 0, 3, 233, // replica 1001 + 0, 0, // empty tagged fields + 7, 116, 111, 112, 105, 99, 50, // topic name "topic2" as compact string + 2, // 2-1=1 partitions + 0, 0, 0, 1, // partitionId + 3, // 3-1=2 replica array size + 0, 0, 3, 233, // replica 1001 + 0, 0, 3, 234, // replica 1002 + 0, 0, // empty tagged fields + 0, // empty tagged fields + } + + alterPartitionReassignmentsRequestTwoPartitions = []byte{ + 0, 0, 39, 16, // timout 10000 + 2, // 2-1=1 block + 6, 116, 111, 112, 105, 99, // topic name "topic" as compact string + 3, // 3-1=2 partitions + 0, 0, 0, 0, // partitionId + 3, // 3-1=2 replica array size + 0, 0, 3, 232, // replica 1000 + 0, 0, 3, 233, // replica 1001 + 0, // empty tagged fields + 0, 0, 0, 1, // partitionId + 3, // 3-1=2 replica array size + 0, 0, 3, 233, // replica 1001 + 0, 0, 3, 234, // replica 1002 + 0, 0, // empty tagged fields + 0, // empty tagged fields + } +) + +func TestAlterPartitionReassignmentRequest(t *testing.T) { + var request *AlterPartitionReassignmentsRequest + + request = &AlterPartitionReassignmentsRequest{ + TimeoutMs: int32(10000), + Version: int16(0), + } + + testRequest(t, "no block", request, alterPartitionReassignmentsRequestNoBlock) + + request.AddBlock("topic", 0, []int32{1000, 1001}) + + testRequest(t, "one block", request, alterPartitionReassignmentsRequestOneBlock) + + request.AddBlock("topic2", 1, []int32{1001, 1002}) + + testRequest(t, "two blocks", request, alterPartitionReassignmentsRequestTwoBlocks) + + request = &AlterPartitionReassignmentsRequest{ + TimeoutMs: int32(10000), + Version: int16(0), + } + + request.AddBlock("topic", 0, []int32{1000, 1001}) + request.AddBlock("topic", 1, []int32{1001, 1002}) + + testRequest(t, "two partitions", request, alterPartitionReassignmentsRequestTwoPartitions) + +} diff --git a/alter_partition_reassignments_response.go b/alter_partition_reassignments_response.go index 3b91a9052..7303bafc6 100644 --- a/alter_partition_reassignments_response.go +++ b/alter_partition_reassignments_response.go @@ -2,12 +2,16 @@ package sarama type alterPartitionReassignmentsErrorBlock struct { errorCode KError - errorMessage string + errorMessage *string } func (b *alterPartitionReassignmentsErrorBlock) encode(pe packetEncoder) error { - pe.putInt32(int32(b.errorCode)) - pe.putString(b.errorMessage) + pe.putInt16(int16(b.errorCode)) + if err := pe.putNullableCompactString(b.errorMessage); err != nil { + return err + } + // tagged field + pe.putUVarint(0) return nil } @@ -18,7 +22,7 @@ func (b *alterPartitionReassignmentsErrorBlock) decode(pd packetDecoder) (err er return err } b.errorCode = KError(errorCode) - b.errorMessage, err = pd.getString() + b.errorMessage, err = pd.getCompactNullableString() return err } @@ -40,31 +44,36 @@ func (r *AlterPartitionReassignmentsResponse) AddError(topic string, partition i r.Errors[topic] = partitions } - partitions[partition] = &alterPartitionReassignmentsErrorBlock{errorCode: kerror, errorMessage: *message} + partitions[partition] = &alterPartitionReassignmentsErrorBlock{errorCode: kerror, errorMessage: message} } func (r *AlterPartitionReassignmentsResponse) encode(pe packetEncoder) error { pe.putInt32(r.ThrottleTimeMs) pe.putInt16(int16(r.ErrorCode)) - if err := pe.putNullableString(r.ErrorMessage); err != nil { + if err := pe.putNullableCompactString(r.ErrorMessage); err != nil { return err } - if err := pe.putArrayLength(len(r.Errors)); err != nil { - return err - } + pe.putCompactArrayLength(len(r.Errors)) for topic, partitions := range r.Errors { - if err := pe.putString(topic); err != nil { - return err - } - if err := pe.putArrayLength(len(partitions)); err != nil { + if err := pe.putCompactString(topic); err != nil { return err } - for partition, kerror := range partitions { + pe.putCompactArrayLength(len(partitions)) + for partition, block := range partitions { pe.putInt32(partition) - pe.putInt16(int16(kerror.errorCode)) + + if err := block.encode(pe); err != nil { + return err + } } + // tagged field + pe.putUVarint(0) } + + // tagged field + pe.putUVarint(0) + return nil } @@ -127,14 +136,23 @@ func (r *AlterPartitionReassignmentsResponse) decode(pd packetDecoder, version i r.AddError(name, partition, KError(errorCode), errorMessage) } - // Tag buffer? - pd.getInt8() + // empty tagged fields array + _, err = pd.getUVarint() + if err != nil { + return err + } + } + // empty tagged fields array + _, err = pd.getUVarint() + if err != nil { + return err } - // Tag buffer? - pd.getInt8() } - // Tag buffer? - pd.getInt8() + // empty tagged fields array + _, err = pd.getUVarint() + if err != nil { + return err + } return nil } diff --git a/alter_partition_reassignments_response_test.go b/alter_partition_reassignments_response_test.go new file mode 100644 index 000000000..39d0aee3f --- /dev/null +++ b/alter_partition_reassignments_response_test.go @@ -0,0 +1,24 @@ +package sarama + +import "testing" + +var ( + alterPartitionReassignmentsResponseNoError = []byte{ + 0, 0, 39, 16, // ThrottleTimeMs 10000 + 0, 0, // errorcode + 0, // null string + 1, // empty errors array + 0, // empty tagged fields + } +) + +func TestAlterPartitionReassignmentResponse(t *testing.T) { + var response *AlterPartitionReassignmentsResponse + + response = &AlterPartitionReassignmentsResponse{ + ThrottleTimeMs: int32(10000), + Version: int16(0), + } + + testResponse(t, "no error", response, alterPartitionReassignmentsResponseNoError) +} diff --git a/packet_encoder.go b/packet_encoder.go index 3fe6bd63a..78784683b 100644 --- a/packet_encoder.go +++ b/packet_encoder.go @@ -13,7 +13,7 @@ type packetEncoder interface { putInt64(in int64) putVarint(in int64) putUVarint(in uint64) - putUVarIntArrayLength(in int) + putCompactArrayLength(in int) putArrayLength(in int) error putBool(in bool) @@ -22,6 +22,7 @@ type packetEncoder interface { putVarintBytes(in []byte) error putRawBytes(in []byte) error putCompactString(in string) error + putNullableCompactString(in *string) error putString(in string) error putNullableString(in *string) error putStringArray(in []string) error diff --git a/prep_encoder.go b/prep_encoder.go index 785af6455..04660daf4 100644 --- a/prep_encoder.go +++ b/prep_encoder.go @@ -49,8 +49,8 @@ func (pe *prepEncoder) putArrayLength(in int) error { return nil } -func (pe *prepEncoder) putUVarIntArrayLength(in int) { - pe.putUVarint(uint64(in+1)) +func (pe *prepEncoder) putCompactArrayLength(in int) { + pe.putUVarint(uint64(in + 1)) } func (pe *prepEncoder) putBool(in bool) { @@ -77,10 +77,19 @@ func (pe *prepEncoder) putVarintBytes(in []byte) error { } func (pe *prepEncoder) putCompactString(in string) error { - pe.putUVarint(uint64(len(in) + 1)) + pe.putCompactArrayLength(len(in)) return pe.putRawBytes([]byte(in)) } +func (pe *prepEncoder) putNullableCompactString(in *string) error { + if in == nil { + pe.putUVarint(0) + return nil + } else { + return pe.putCompactString(*in) + } +} + func (pe *prepEncoder) putRawBytes(in []byte) error { if len(in) > math.MaxInt32 { return PacketEncodingError{fmt.Sprintf("byteslice too long (%d)", len(in))} @@ -122,7 +131,7 @@ func (pe *prepEncoder) putStringArray(in []string) error { } func (pe *prepEncoder) putCompactInt32Array(in []int32) error { - pe.putUVarint(uint64(len(in))+1) + pe.putUVarint(uint64(len(in)) + 1) pe.length += 4 * len(in) return nil } diff --git a/real_encoder.go b/real_encoder.go index 4ba962258..53b08ae9c 100644 --- a/real_encoder.go +++ b/real_encoder.go @@ -48,7 +48,8 @@ func (re *realEncoder) putArrayLength(in int) error { return nil } -func (re *realEncoder) putUVarIntArrayLength(in int) { +func (re *realEncoder) putCompactArrayLength(in int) { + // 0 represents a null array, so +1 has to be added re.putUVarint(uint64(in + 1)) } @@ -87,10 +88,18 @@ func (re *realEncoder) putVarintBytes(in []byte) error { } func (re *realEncoder) putCompactString(in string) error { - re.putUVarint(uint64(len(in) + 1)) + re.putCompactArrayLength(len(in)) return re.putRawBytes([]byte(in)) } +func (re *realEncoder) putNullableCompactString(in *string) error { + if in == nil { + re.putInt8(0) + return nil + } + return re.putCompactString(*in) +} + func (re *realEncoder) putString(in string) error { re.putInt16(int16(len(in))) copy(re.raw[re.off:], in) @@ -122,6 +131,7 @@ func (re *realEncoder) putStringArray(in []string) error { } func (re *realEncoder) putCompactInt32Array(in []int32) error { + // 0 represents a null array, so +1 has to be added re.putUVarint(uint64(len(in)) + 1) for _, val := range in { re.putInt32(val) diff --git a/request.go b/request.go index 62b8913ed..fbbb7f108 100644 --- a/request.go +++ b/request.go @@ -36,7 +36,7 @@ func (r *request) encode(pe packetEncoder) error { if r.body.headerVersion() >= 2 { // we don't use tag headers at the moment so we just put an array length of 0 - pe.putUVarint(0); + pe.putUVarint(0) } err := r.body.encode(pe) @@ -73,6 +73,14 @@ func (r *request) decode(pd packetDecoder) (err error) { return PacketDecodingError{fmt.Sprintf("unknown request key (%d)", key)} } + if r.body.headerVersion() >= 2 { + // tagged field + _, err = pd.getUVarint() + if err != nil { + return err + } + } + return r.body.decode(pd, version) } diff --git a/request_test.go b/request_test.go index fec190795..fdfe79eab 100644 --- a/request_test.go +++ b/request_test.go @@ -45,7 +45,18 @@ func testRequest(t *testing.T, name string, rb protocolBody, expected []byte) { func testRequestEncode(t *testing.T, name string, rb protocolBody, expected []byte) []byte { req := &request{correlationID: 123, clientID: "foo", body: rb} packet, err := encode(req, nil) - headerSize := 14 + len("foo") + + headerSize := 0 + + switch rb.headerVersion() { + case 1: + headerSize = 14 + len("foo") + case 2: + headerSize = 14 + len("foo") + 1 + default: + t.Error("Encoding", name, "failed\nheaderVersion", rb.headerVersion(), "not implemented") + } + if err != nil { t.Error(err) } else if !bytes.Equal(packet[headerSize:], expected) { From 675d2830688a34a75b7ee27d271cb6452f1f01f6 Mon Sep 17 00:00:00 2001 From: Dirk Wilden Date: Thu, 13 Feb 2020 14:09:22 +0100 Subject: [PATCH 05/20] add test for alter_partition_reassignments_response --- ...r_partition_reassignments_response_test.go | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/alter_partition_reassignments_response_test.go b/alter_partition_reassignments_response_test.go index 39d0aee3f..614b571b3 100644 --- a/alter_partition_reassignments_response_test.go +++ b/alter_partition_reassignments_response_test.go @@ -10,6 +10,19 @@ var ( 1, // empty errors array 0, // empty tagged fields } + + alterPartitionReassignmentsResponseWithError = []byte{ + 0, 0, 39, 16, // ThrottleTimeMs 10000 + 0, 12, // errorcode + 6, 101, 114, 114, 111, 114, // error string "error" + 2, // errors array length 1 + 6, 116, 111, 112, 105, 99, // topic name "topic" + 2, // partition array length 1 + 0, 0, 0, 1, // partitionId + 0, 3, //kerror + 7, 101, 114, 114, 111, 114, 50, // error string "error2" + 0, 0, 0, // empty tagged fields + } ) func TestAlterPartitionReassignmentResponse(t *testing.T) { @@ -21,4 +34,12 @@ func TestAlterPartitionReassignmentResponse(t *testing.T) { } testResponse(t, "no error", response, alterPartitionReassignmentsResponseNoError) + + errorMessage := "error" + partitionError := "error2" + response.ErrorCode = 12 + response.ErrorMessage = &errorMessage + response.AddError("topic", 1, 3, &partitionError) + + testResponse(t, "with error", response, alterPartitionReassignmentsResponseWithError) } From e2b1df8812b952b49de4858feb9618dea5bb6e49 Mon Sep 17 00:00:00 2001 From: sladkoff Date: Thu, 13 Feb 2020 15:54:36 +0100 Subject: [PATCH 06/20] Pretty print partition reassignment errors --- admin.go | 24 ++++++++++++++++++++---- errors.go | 16 ++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/admin.go b/admin.go index 298a866a6..ac8d32401 100644 --- a/admin.go +++ b/admin.go @@ -470,15 +470,31 @@ func (ca *clusterAdmin) AlterPartitionReassignments(topic string, assignment [][ return err } + errs := make([]error, 0) + rsp, err := b.AlterPartitionReassignments(request) if err != nil { - return err + errs = append(errs, err) + } else { + + if rsp.ErrorCode > 0 { + errs = append(errs, errors.New(rsp.ErrorCode.Error())) + } + + for topic, topicErrors := range rsp.Errors { + for partition, partitionError := range topicErrors { + if partitionError.errorCode != ErrNoError { + errStr := fmt.Sprintf("[%s-%d]: %s", topic, partition, partitionError.errorCode.Error()) + errs = append(errs, errors.New(errStr)) + } + } + } + } - if rsp.ErrorCode > 0 || len(rsp.Errors) > 0 { - // TODO pretty error from response - return errors.New("Kafka server returned errors during replication-factor assignment") + if len(errs) > 0 { + return ErrReassignPartitions{MultiError{&errs}} } return nil diff --git a/errors.go b/errors.go index 97be3c0f1..ca621b092 100644 --- a/errors.go +++ b/errors.go @@ -94,6 +94,14 @@ func (mErr MultiError) Error() string { return errString } +func (mErr MultiError) PrettyError() string { + var errString = "" + for _, err := range *mErr.Errors { + errString += err.Error() + "\n" + } + return errString +} + // ErrDeleteRecords is the type of error returned when fail to delete the required records type ErrDeleteRecords struct { MultiError @@ -103,6 +111,14 @@ func (err ErrDeleteRecords) Error() string { return "kafka server: failed to delete records " + err.MultiError.Error() } +type ErrReassignPartitions struct { + MultiError +} + +func (err ErrReassignPartitions) Error() string { + return fmt.Sprintf("failed to reassign partitions for topic: \n%s", err.MultiError.PrettyError()) +} + // Numeric error codes returned by the Kafka server. const ( ErrNoError KError = 0 From 15d7fb3bac1139145a2b6a9e27f2dd8a92a7ca0d Mon Sep 17 00:00:00 2001 From: sladkoff Date: Thu, 13 Feb 2020 16:49:13 +0100 Subject: [PATCH 07/20] Fix wrong offset increment --- real_decoder.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/real_decoder.go b/real_decoder.go index 58632ccc4..cf31ad209 100644 --- a/real_decoder.go +++ b/real_decoder.go @@ -9,6 +9,7 @@ var errInvalidArrayLength = PacketDecodingError{"invalid array length"} var errInvalidByteSliceLength = PacketDecodingError{"invalid byteslice length"} var errInvalidStringLength = PacketDecodingError{"invalid string length"} var errVarintOverflow = PacketDecodingError{"varint overflow"} +var errUVarintOverflow = PacketDecodingError{"uvarint overflow"} var errInvalidBool = PacketDecodingError{"invalid bool"} type realDecoder struct { @@ -79,10 +80,12 @@ func (rd *realDecoder) getUVarint() (uint64, error) { rd.off = int(len(rd.raw)) return 0, ErrInsufficientData } + if n < 0 { rd.off -= n - return 0, errVarintOverflow + return 0, errUVarintOverflow } + rd.off += n return tmp, nil } @@ -217,7 +220,6 @@ func (rd *realDecoder) getCompactNullableString() (*string, error) { var length = int(n - 1) if length < 0 { - rd.off += 1 return nil, err } From cd507030b94d38056d9ab520a82111f715d66e95 Mon Sep 17 00:00:00 2001 From: sladkoff Date: Thu, 13 Feb 2020 17:12:31 +0100 Subject: [PATCH 08/20] Add admin tests for AlterPartitionReassignments --- admin.go | 1 - admin_test.go | 63 ++++++++++++++++++++++++++++++++++++++++++++++++ mockresponses.go | 15 ++++++++++++ 3 files changed, 78 insertions(+), 1 deletion(-) diff --git a/admin.go b/admin.go index ac8d32401..fe440c041 100644 --- a/admin.go +++ b/admin.go @@ -490,7 +490,6 @@ func (ca *clusterAdmin) AlterPartitionReassignments(topic string, assignment [][ } } } - } if len(errs) > 0 { diff --git a/admin_test.go b/admin_test.go index a6a05bf33..da09c7018 100644 --- a/admin_test.go +++ b/admin_test.go @@ -332,6 +332,69 @@ func TestClusterAdminCreatePartitionsWithoutAuthorization(t *testing.T) { } } +func TestClusterAdminAlterPartitionReassignments(t *testing.T) { + seedBroker := NewMockBroker(t, 1) + defer seedBroker.Close() + + seedBroker.SetHandlerByMap(map[string]MockResponse{ + "MetadataRequest": NewMockMetadataResponse(t). + SetController(seedBroker.BrokerID()). + SetBroker(seedBroker.Addr(), seedBroker.BrokerID()), + "AlterPartitionReassignmentsRequest": NewMockAlterPartitionReassignmentsResponse(t), + }) + + config := NewConfig() + config.Version = V2_4_0_0 + admin, err := NewClusterAdmin([]string{seedBroker.Addr()}, config) + if err != nil { + t.Fatal(err) + } + + var topicAssignment = make([][]int32, 0, 3) + + err = admin.AlterPartitionReassignments("my_topic", topicAssignment) + if err != nil { + t.Fatal(err) + } + + err = admin.Close() + if err != nil { + t.Fatal(err) + } +} + +func TestClusterAdminAlterPartitionReassignmentsWithDiffVersion(t *testing.T) { + seedBroker := NewMockBroker(t, 1) + defer seedBroker.Close() + + seedBroker.SetHandlerByMap(map[string]MockResponse{ + "MetadataRequest": NewMockMetadataResponse(t). + SetController(seedBroker.BrokerID()). + SetBroker(seedBroker.Addr(), seedBroker.BrokerID()), + "AlterPartitionReassignmentsRequest": NewMockAlterPartitionReassignmentsResponse(t), + }) + + config := NewConfig() + config.Version = V2_3_0_0 + admin, err := NewClusterAdmin([]string{seedBroker.Addr()}, config) + if err != nil { + t.Fatal(err) + } + + var topicAssignment = make([][]int32, 0, 3) + + err = admin.AlterPartitionReassignments("my_topic", topicAssignment) + + if !strings.ContainsAny(err.Error(), ErrUnsupportedVersion.Error()) { + t.Fatal(err) + } + + err = admin.Close() + if err != nil { + t.Fatal(err) + } +} + func TestClusterAdminDeleteRecords(t *testing.T) { topicName := "my_topic" seedBroker := NewMockBroker(t, 1) diff --git a/mockresponses.go b/mockresponses.go index a7ba44b10..8c8cd055e 100644 --- a/mockresponses.go +++ b/mockresponses.go @@ -698,6 +698,21 @@ func (mr *MockCreatePartitionsResponse) For(reqBody versionedDecoder) encoder { return res } +type MockAlterPartitionReassignmentsResponse struct { + t TestReporter +} + +func NewMockAlterPartitionReassignmentsResponse(t TestReporter) *MockAlterPartitionReassignmentsResponse { + return &MockAlterPartitionReassignmentsResponse{t: t} +} + +func (mr *MockAlterPartitionReassignmentsResponse) For(reqBody versionedDecoder) encoder { + req := reqBody.(*AlterPartitionReassignmentsRequest) + _ = req + res := &AlterPartitionReassignmentsResponse{} + return res +} + type MockDeleteRecordsResponse struct { t TestReporter } From e4f9b4c530e166031176f4a1057789cbf143e603 Mon Sep 17 00:00:00 2001 From: Dirk Wilden Date: Fri, 14 Feb 2020 08:59:11 +0100 Subject: [PATCH 09/20] add ListPartitionReassignmentsRequest --- list_partition_reassignments_request.go | 104 +++++++++++++++++++ list_partition_reassignments_request_test.go | 45 ++++++++ request.go | 2 + 3 files changed, 151 insertions(+) create mode 100644 list_partition_reassignments_request.go create mode 100644 list_partition_reassignments_request_test.go diff --git a/list_partition_reassignments_request.go b/list_partition_reassignments_request.go new file mode 100644 index 000000000..7363f3ed6 --- /dev/null +++ b/list_partition_reassignments_request.go @@ -0,0 +1,104 @@ +package sarama + +type ListPartitionReassignmentsRequest struct { + TimeoutMs int32 + blocks map[string][]int32 + Version int16 +} + +func (r *ListPartitionReassignmentsRequest) encode(pe packetEncoder) error { + pe.putInt32(r.TimeoutMs) + + pe.putCompactArrayLength(len(r.blocks)) + + for topic, partitions := range r.blocks { + if err := pe.putCompactString(topic); err != nil { + return err + } + + if err := pe.putCompactInt32Array(partitions); err != nil { + return err + } + + //another tagged field + pe.putInt8(0) + } + + //another tagged field + pe.putInt8(0) + + return nil +} + +func (r *ListPartitionReassignmentsRequest) decode(pd packetDecoder, version int16) (err error) { + r.Version = version + + if r.TimeoutMs, err = pd.getInt32(); err != nil { + return err + } + + topicCount, err := pd.getCompactArrayLength() + if err != nil { + return err + } + if topicCount > 0 { + r.blocks = make(map[string][]int32) + for i := 0; i < topicCount; i++ { + topic, err := pd.getCompactString() + if err != nil { + return err + } + partitionCount, err := pd.getCompactArrayLength() + if err != nil { + return err + } + r.blocks[topic] = make([]int32, partitionCount) + for j := 0; j < partitionCount; j++ { + partition, err := pd.getInt32() + if err != nil { + return err + } + r.blocks[topic][j] = partition + } + // empty tagged fields array + _, err = pd.getUVarint() + if err != nil { + return err + } + } + } + + // empty tagged fields array + _, err = pd.getUVarint() + if err != nil { + return err + } + + return +} + +func (r *ListPartitionReassignmentsRequest) key() int16 { + return 46 +} + +func (r *ListPartitionReassignmentsRequest) version() int16 { + return r.Version +} + +func (r *ListPartitionReassignmentsRequest) headerVersion() int16 { + return 2 +} + +func (r *ListPartitionReassignmentsRequest) requiredVersion() KafkaVersion { + return V2_4_0_0 +} + +func (r *ListPartitionReassignmentsRequest) AddBlock(topic string, partitionIDs []int32) { + if r.blocks == nil { + r.blocks = make(map[string][]int32) + } + + if r.blocks[topic] == nil { + r.blocks[topic] = partitionIDs + } +} diff --git a/list_partition_reassignments_request_test.go b/list_partition_reassignments_request_test.go new file mode 100644 index 000000000..d50b35a26 --- /dev/null +++ b/list_partition_reassignments_request_test.go @@ -0,0 +1,45 @@ +package sarama + +import "testing" + +var ( + listPartitionReassignmentsRequestOneBlock = []byte{ + 0, 0, 39, 16, // timout 10000 + 2, // 2-1=1 block + 6, 116, 111, 112, 105, 99, // topic name "topic" as compact string + 2, // 2-1=1 partitions + 0, 0, 0, 0, // partitionId + 0, 0, // empty tagged fields + } + + listPartitionReassignmentsRequestTwoBlocks = []byte{ + 0, 0, 39, 16, // timout 10000 + 3, // 3-1=2 blocks + 6, 116, 111, 112, 105, 99, // topic name "topic" as compact string + 2, // 2-1=1 partitions + 0, 0, 0, 0, // partitionId + 0, // empty tagged fields + 7, 116, 111, 112, 105, 99, 50, // topic name "topic2" as compact string + 3, // 3-1=2 partitions + 0, 0, 0, 1, // partitionId + 0, 0, 0, 2, // partitionId + 0, 0, // empty tagged fields + } +) + +func TestListPartitionReassignmentRequest(t *testing.T) { + var request *ListPartitionReassignmentsRequest + + request = &ListPartitionReassignmentsRequest{ + TimeoutMs: int32(10000), + Version: int16(0), + } + + request.AddBlock("topic", []int32{0}) + + testRequest(t, "one block", request, listPartitionReassignmentsRequestOneBlock) + + request.AddBlock("topic2", []int32{1, 2}) + + testRequest(t, "two blocks", request, listPartitionReassignmentsRequestTwoBlocks) +} diff --git a/request.go b/request.go index fbbb7f108..dcfd3946c 100644 --- a/request.go +++ b/request.go @@ -184,6 +184,8 @@ func allocateBody(key, version int16) protocolBody { return &DeleteGroupsRequest{} case 45: return &AlterPartitionReassignmentsRequest{} + case 46: + return &ListPartitionReassignmentsRequest{} } return nil } From bf390fb9328111b38b3e8f1b15faa2a5e8816420 Mon Sep 17 00:00:00 2001 From: sladkoff Date: Fri, 14 Feb 2020 12:51:07 +0100 Subject: [PATCH 10/20] Decode empty tagged fields in response header v1 --- acl_create_response.go | 2 +- acl_delete_response.go | 2 +- acl_describe_response.go | 2 +- add_offsets_to_txn_response.go | 2 +- add_partitions_to_txn_response.go | 2 +- alter_configs_response.go | 2 +- alter_partition_reassignments_response.go | 14 +++------- api_versions_response.go | 2 +- broker.go | 31 ++++++++++++++++------- consumer_metadata_response.go | 2 +- create_partitions_response.go | 2 +- create_topics_response.go | 2 +- delete_groups_response.go | 2 +- delete_records_response.go | 2 +- delete_topics_response.go | 2 +- describe_configs_response.go | 2 +- describe_groups_response.go | 2 +- describe_log_dirs_response.go | 2 +- end_txn_response.go | 2 +- fetch_response.go | 2 +- find_coordinator_response.go | 2 +- heartbeat_response.go | 2 +- init_producer_id_response.go | 2 +- join_group_response.go | 2 +- leave_group_response.go | 2 +- list_groups_response.go | 2 +- metadata_response.go | 2 +- offset_commit_response.go | 2 +- offset_fetch_response.go | 2 +- offset_response.go | 2 +- packet_decoder.go | 1 + produce_response.go | 16 ++++++++++++ real_decoder.go | 14 ++++++++++ response_header.go | 9 ++++++- sasl_authenticate_response.go | 2 +- sasl_handshake_response.go | 2 +- sync_group_response.go | 2 +- txn_offset_commit_response.go | 2 +- 38 files changed, 97 insertions(+), 52 deletions(-) diff --git a/acl_create_response.go b/acl_create_response.go index 499616781..bc018ed00 100644 --- a/acl_create_response.go +++ b/acl_create_response.go @@ -56,7 +56,7 @@ func (c *CreateAclsResponse) version() int16 { } func (c *CreateAclsResponse) headerVersion() int16 { - return 1 + return 0 } func (c *CreateAclsResponse) requiredVersion() KafkaVersion { diff --git a/acl_delete_response.go b/acl_delete_response.go index f74f23761..cb6308826 100644 --- a/acl_delete_response.go +++ b/acl_delete_response.go @@ -57,7 +57,7 @@ func (d *DeleteAclsResponse) version() int16 { } func (d *DeleteAclsResponse) headerVersion() int16 { - return 1 + return 0 } func (d *DeleteAclsResponse) requiredVersion() KafkaVersion { diff --git a/acl_describe_response.go b/acl_describe_response.go index 7dadc2b46..c43408b24 100644 --- a/acl_describe_response.go +++ b/acl_describe_response.go @@ -78,7 +78,7 @@ func (d *DescribeAclsResponse) version() int16 { } func (d *DescribeAclsResponse) headerVersion() int16 { - return 1 + return 0 } func (d *DescribeAclsResponse) requiredVersion() KafkaVersion { diff --git a/add_offsets_to_txn_response.go b/add_offsets_to_txn_response.go index cf49ba642..bdb184419 100644 --- a/add_offsets_to_txn_response.go +++ b/add_offsets_to_txn_response.go @@ -41,7 +41,7 @@ func (a *AddOffsetsToTxnResponse) version() int16 { } func (a *AddOffsetsToTxnResponse) headerVersion() int16 { - return 1 + return 0 } func (a *AddOffsetsToTxnResponse) requiredVersion() KafkaVersion { diff --git a/add_partitions_to_txn_response.go b/add_partitions_to_txn_response.go index 4dc2d7551..73b73b07f 100644 --- a/add_partitions_to_txn_response.go +++ b/add_partitions_to_txn_response.go @@ -80,7 +80,7 @@ func (a *AddPartitionsToTxnResponse) version() int16 { } func (a *AddPartitionsToTxnResponse) headerVersion() int16 { - return 1 + return 0 } func (a *AddPartitionsToTxnResponse) requiredVersion() KafkaVersion { diff --git a/alter_configs_response.go b/alter_configs_response.go index 7fa5c5db0..99a526005 100644 --- a/alter_configs_response.go +++ b/alter_configs_response.go @@ -93,7 +93,7 @@ func (a *AlterConfigsResponse) version() int16 { } func (a *AlterConfigsResponse) headerVersion() int16 { - return 1 + return 0 } func (a *AlterConfigsResponse) requiredVersion() KafkaVersion { diff --git a/alter_partition_reassignments_response.go b/alter_partition_reassignments_response.go index 7303bafc6..30538d91e 100644 --- a/alter_partition_reassignments_response.go +++ b/alter_partition_reassignments_response.go @@ -136,21 +136,15 @@ func (r *AlterPartitionReassignmentsResponse) decode(pd packetDecoder, version i r.AddError(name, partition, KError(errorCode), errorMessage) } - // empty tagged fields array - _, err = pd.getUVarint() - if err != nil { + if _, err = pd.getEmptyTaggedFields(); err != nil { return err } } - // empty tagged fields array - _, err = pd.getUVarint() - if err != nil { + if _, err = pd.getEmptyTaggedFields(); err != nil { return err } } - // empty tagged fields array - _, err = pd.getUVarint() - if err != nil { + if _, err = pd.getEmptyTaggedFields(); err != nil { return err } @@ -166,7 +160,7 @@ func (r *AlterPartitionReassignmentsResponse) version() int16 { } func (r *AlterPartitionReassignmentsResponse) headerVersion() int16 { - return 2 + return 1 } func (r *AlterPartitionReassignmentsResponse) requiredVersion() KafkaVersion { diff --git a/api_versions_response.go b/api_versions_response.go index 66bd66c77..582e29db4 100644 --- a/api_versions_response.go +++ b/api_versions_response.go @@ -85,7 +85,7 @@ func (r *ApiVersionsResponse) version() int16 { } func (a *ApiVersionsResponse) headerVersion() int16 { - return 1 + return 0 } func (r *ApiVersionsResponse) requiredVersion() KafkaVersion { diff --git a/broker.go b/broker.go index 86de2e11c..723c4c920 100644 --- a/broker.go +++ b/broker.go @@ -119,6 +119,7 @@ type SCRAMClient interface { type responsePromise struct { requestTime time.Time correlationID int32 + headerVersion int16 packets chan []byte errors chan error } @@ -706,7 +707,7 @@ func (b *Broker) write(buf []byte) (n int, err error) { return b.conn.Write(buf) } -func (b *Broker) send(rb protocolBody, promiseResponse bool) (*responsePromise, error) { +func (b *Broker) send(rb protocolBody, promiseResponse bool, responseHeaderVersion int16) (*responsePromise, error) { b.lock.Lock() defer b.lock.Unlock() @@ -744,14 +745,14 @@ func (b *Broker) send(rb protocolBody, promiseResponse bool) (*responsePromise, return nil, nil } - promise := responsePromise{requestTime, req.correlationID, make(chan []byte), make(chan error)} + promise := responsePromise{requestTime, req.correlationID, responseHeaderVersion, make(chan []byte), make(chan error)} b.responses <- promise return &promise, nil } -func (b *Broker) sendAndReceive(req protocolBody, res versionedDecoder) error { - promise, err := b.send(req, res != nil) +func (b *Broker) sendAndReceive(req protocolBody, res protocolBody) error { + promise, err := b.send(req, res != nil, res.headerVersion()) if err != nil { return err } @@ -831,7 +832,6 @@ func (b *Broker) encode(pe packetEncoder, version int16) (err error) { func (b *Broker) responseReceiver() { var dead error - header := make([]byte, 8) for response := range b.responses { if dead != nil { @@ -842,6 +842,9 @@ func (b *Broker) responseReceiver() { continue } + var headerLength = getHeaderLength(response.headerVersion) + header := make([]byte, headerLength) + bytesReadHeader, err := b.readFull(header) requestLatency := time.Since(response.requestTime) if err != nil { @@ -852,7 +855,7 @@ func (b *Broker) responseReceiver() { } decodedHeader := responseHeader{} - err = decode(header, &decodedHeader) + err = versionedDecode(header, &decodedHeader, response.headerVersion) if err != nil { b.updateIncomingCommunicationMetrics(bytesReadHeader, requestLatency) dead = err @@ -868,7 +871,7 @@ func (b *Broker) responseReceiver() { continue } - buf := make([]byte, decodedHeader.length-4) + buf := make([]byte, decodedHeader.length-int32(headerLength)+4) bytesReadBody, err := b.readFull(buf) b.updateIncomingCommunicationMetrics(bytesReadHeader+bytesReadBody, requestLatency) if err != nil { @@ -882,6 +885,16 @@ func (b *Broker) responseReceiver() { close(b.done) } +func getHeaderLength(headerVersion int16) int8 { + + if headerVersion < 1 { + return 8 + } else { + // header contains additional tagged field length (0), we don't support actual tags yet. + return 9 + } +} + func (b *Broker) authenticateViaSASL() error { switch b.conf.Net.SASL.Mechanism { case SASLTypeOAuth: @@ -1193,7 +1206,7 @@ func (b *Broker) receiveSaslAuthenticateResponse(correlationID int32) ([]byte, e } header := responseHeader{} - err = decode(buf, &header) + err = versionedDecode(buf, &header, 1) if err != nil { return nil, err } @@ -1282,7 +1295,7 @@ func (b *Broker) receiveSASLServerResponse(res *SaslAuthenticateResponse, correl } header := responseHeader{} - err = decode(buf, &header) + err = versionedDecode(buf, &header, 1) if err != nil { return bytesRead, err } diff --git a/consumer_metadata_response.go b/consumer_metadata_response.go index 0e08549ed..1b5d00d22 100644 --- a/consumer_metadata_response.go +++ b/consumer_metadata_response.go @@ -74,7 +74,7 @@ func (r *ConsumerMetadataResponse) version() int16 { } func (r *ConsumerMetadataResponse) headerVersion() int16 { - return 1 + return 0 } func (r *ConsumerMetadataResponse) requiredVersion() KafkaVersion { diff --git a/create_partitions_response.go b/create_partitions_response.go index f15a2433c..12ce78857 100644 --- a/create_partitions_response.go +++ b/create_partitions_response.go @@ -64,7 +64,7 @@ func (r *CreatePartitionsResponse) version() int16 { } func (r *CreatePartitionsResponse) headerVersion() int16 { - return 1 + return 0 } func (r *CreatePartitionsResponse) requiredVersion() KafkaVersion { diff --git a/create_topics_response.go b/create_topics_response.go index a600fc419..7e1448a66 100644 --- a/create_topics_response.go +++ b/create_topics_response.go @@ -71,7 +71,7 @@ func (c *CreateTopicsResponse) version() int16 { } func (c *CreateTopicsResponse) headerVersion() int16 { - return 1 + return 0 } func (c *CreateTopicsResponse) requiredVersion() KafkaVersion { diff --git a/delete_groups_response.go b/delete_groups_response.go index 99e4da6d7..5e7b1ed36 100644 --- a/delete_groups_response.go +++ b/delete_groups_response.go @@ -66,7 +66,7 @@ func (r *DeleteGroupsResponse) version() int16 { } func (r *DeleteGroupsResponse) headerVersion() int16 { - return 1 + return 0 } func (r *DeleteGroupsResponse) requiredVersion() KafkaVersion { diff --git a/delete_records_response.go b/delete_records_response.go index 156eb6a17..d530b4c7e 100644 --- a/delete_records_response.go +++ b/delete_records_response.go @@ -81,7 +81,7 @@ func (d *DeleteRecordsResponse) version() int16 { } func (d *DeleteRecordsResponse) headerVersion() int16 { - return 1 + return 0 } func (d *DeleteRecordsResponse) requiredVersion() KafkaVersion { diff --git a/delete_topics_response.go b/delete_topics_response.go index 1ce489b10..733961a89 100644 --- a/delete_topics_response.go +++ b/delete_topics_response.go @@ -69,7 +69,7 @@ func (d *DeleteTopicsResponse) version() int16 { } func (d *DeleteTopicsResponse) headerVersion() int16 { - return 1 + return 0 } func (d *DeleteTopicsResponse) requiredVersion() KafkaVersion { diff --git a/describe_configs_response.go b/describe_configs_response.go index 844efefb9..533e568e5 100644 --- a/describe_configs_response.go +++ b/describe_configs_response.go @@ -113,7 +113,7 @@ func (r *DescribeConfigsResponse) version() int16 { } func (r *DescribeConfigsResponse) headerVersion() int16 { - return 1 + return 0 } func (r *DescribeConfigsResponse) requiredVersion() KafkaVersion { diff --git a/describe_groups_response.go b/describe_groups_response.go index 349490855..bc242e421 100644 --- a/describe_groups_response.go +++ b/describe_groups_response.go @@ -44,7 +44,7 @@ func (r *DescribeGroupsResponse) version() int16 { } func (r *DescribeGroupsResponse) headerVersion() int16 { - return 1 + return 0 } func (r *DescribeGroupsResponse) requiredVersion() KafkaVersion { diff --git a/describe_log_dirs_response.go b/describe_log_dirs_response.go index 7691535cf..a9a747615 100644 --- a/describe_log_dirs_response.go +++ b/describe_log_dirs_response.go @@ -62,7 +62,7 @@ func (r *DescribeLogDirsResponse) version() int16 { } func (r *DescribeLogDirsResponse) headerVersion() int16 { - return 1 + return 0 } func (r *DescribeLogDirsResponse) requiredVersion() KafkaVersion { diff --git a/end_txn_response.go b/end_txn_response.go index 8d6acf5eb..763976726 100644 --- a/end_txn_response.go +++ b/end_txn_response.go @@ -40,7 +40,7 @@ func (e *EndTxnResponse) version() int16 { } func (r *EndTxnResponse) headerVersion() int16 { - return 1 + return 0 } func (e *EndTxnResponse) requiredVersion() KafkaVersion { diff --git a/fetch_response.go b/fetch_response.go index eee282c30..ca6d78832 100644 --- a/fetch_response.go +++ b/fetch_response.go @@ -336,7 +336,7 @@ func (r *FetchResponse) version() int16 { } func (r *FetchResponse) headerVersion() int16 { - return 1 + return 0 } func (r *FetchResponse) requiredVersion() KafkaVersion { diff --git a/find_coordinator_response.go b/find_coordinator_response.go index b2971c9b4..83a648ad4 100644 --- a/find_coordinator_response.go +++ b/find_coordinator_response.go @@ -83,7 +83,7 @@ func (f *FindCoordinatorResponse) version() int16 { } func (r *FindCoordinatorResponse) headerVersion() int16 { - return 1 + return 0 } func (f *FindCoordinatorResponse) requiredVersion() KafkaVersion { diff --git a/heartbeat_response.go b/heartbeat_response.go index 6589a2dfb..577ab72e5 100644 --- a/heartbeat_response.go +++ b/heartbeat_response.go @@ -28,7 +28,7 @@ func (r *HeartbeatResponse) version() int16 { } func (r *HeartbeatResponse) headerVersion() int16 { - return 1 + return 0 } func (r *HeartbeatResponse) requiredVersion() KafkaVersion { diff --git a/init_producer_id_response.go b/init_producer_id_response.go index 5c4aca2c3..3e1242bf6 100644 --- a/init_producer_id_response.go +++ b/init_producer_id_response.go @@ -51,7 +51,7 @@ func (i *InitProducerIDResponse) version() int16 { } func (i *InitProducerIDResponse) headerVersion() int16 { - return 1 + return 0 } func (i *InitProducerIDResponse) requiredVersion() KafkaVersion { diff --git a/join_group_response.go b/join_group_response.go index bebafbf32..54b0a45c2 100644 --- a/join_group_response.go +++ b/join_group_response.go @@ -124,7 +124,7 @@ func (r *JoinGroupResponse) version() int16 { } func (r *JoinGroupResponse) headerVersion() int16 { - return 1 + return 0 } func (r *JoinGroupResponse) requiredVersion() KafkaVersion { diff --git a/leave_group_response.go b/leave_group_response.go index 4544e2e08..25f8d5eb3 100644 --- a/leave_group_response.go +++ b/leave_group_response.go @@ -28,7 +28,7 @@ func (r *LeaveGroupResponse) version() int16 { } func (r *LeaveGroupResponse) headerVersion() int16 { - return 1 + return 0 } func (r *LeaveGroupResponse) requiredVersion() KafkaVersion { diff --git a/list_groups_response.go b/list_groups_response.go index 7b1c495a1..777bae7e6 100644 --- a/list_groups_response.go +++ b/list_groups_response.go @@ -65,7 +65,7 @@ func (r *ListGroupsResponse) version() int16 { } func (r *ListGroupsResponse) headerVersion() int16 { - return 1 + return 0 } func (r *ListGroupsResponse) requiredVersion() KafkaVersion { diff --git a/metadata_response.go b/metadata_response.go index c6b43d7ca..0bb8702cc 100644 --- a/metadata_response.go +++ b/metadata_response.go @@ -256,7 +256,7 @@ func (r *MetadataResponse) version() int16 { } func (r *MetadataResponse) headerVersion() int16 { - return 1 + return 0 } func (r *MetadataResponse) requiredVersion() KafkaVersion { diff --git a/offset_commit_response.go b/offset_commit_response.go index 8648a49df..342260ef5 100644 --- a/offset_commit_response.go +++ b/offset_commit_response.go @@ -95,7 +95,7 @@ func (r *OffsetCommitResponse) version() int16 { } func (r *OffsetCommitResponse) headerVersion() int16 { - return 1 + return 0 } func (r *OffsetCommitResponse) requiredVersion() KafkaVersion { diff --git a/offset_fetch_response.go b/offset_fetch_response.go index 5632e68cc..9c64e0708 100644 --- a/offset_fetch_response.go +++ b/offset_fetch_response.go @@ -156,7 +156,7 @@ func (r *OffsetFetchResponse) version() int16 { } func (r *OffsetFetchResponse) headerVersion() int16 { - return 1 + return 0 } func (r *OffsetFetchResponse) requiredVersion() KafkaVersion { diff --git a/offset_response.go b/offset_response.go index 0fdfa2281..ead3ebbcc 100644 --- a/offset_response.go +++ b/offset_response.go @@ -151,7 +151,7 @@ func (r *OffsetResponse) version() int16 { } func (r *OffsetResponse) headerVersion() int16 { - return 1 + return 0 } func (r *OffsetResponse) requiredVersion() KafkaVersion { diff --git a/packet_decoder.go b/packet_decoder.go index bd95a3b02..5e0b0a07f 100644 --- a/packet_decoder.go +++ b/packet_decoder.go @@ -14,6 +14,7 @@ type packetDecoder interface { getArrayLength() (int, error) getCompactArrayLength() (int, error) getBool() (bool, error) + getEmptyTaggedFields() (int, error) // Collections getBytes() ([]byte, error) diff --git a/produce_response.go b/produce_response.go index 7fc2fffb3..edf978790 100644 --- a/produce_response.go +++ b/produce_response.go @@ -163,6 +163,22 @@ func (r *ProduceResponse) encode(pe packetEncoder) error { return nil } +func (r *ProduceResponse) key() int16 { + return 0 +} + +func (r *ProduceResponse) version() int16 { + return r.Version +} + +func (r *ProduceResponse) headerVersion() int16 { + return 0 +} + +func (r *ProduceResponse) requiredVersion() KafkaVersion { + return MinVersion +} + func (r *ProduceResponse) GetBlock(topic string, partition int32) *ProduceResponseBlock { if r.Blocks == nil { return nil diff --git a/real_decoder.go b/real_decoder.go index cf31ad209..4ebad2d89 100644 --- a/real_decoder.go +++ b/real_decoder.go @@ -11,6 +11,7 @@ var errInvalidStringLength = PacketDecodingError{"invalid string length"} var errVarintOverflow = PacketDecodingError{"varint overflow"} var errUVarintOverflow = PacketDecodingError{"uvarint overflow"} var errInvalidBool = PacketDecodingError{"invalid bool"} +var errUnsupportedTaggedFields = PacketDecodingError{"non-empty tagged fields are not supported yet"} type realDecoder struct { raw []byte @@ -130,6 +131,19 @@ func (rd *realDecoder) getBool() (bool, error) { return true, nil } +func (rd *realDecoder) getEmptyTaggedFields() (int, error) { + tagCount, err := rd.getUVarint() + if err != nil { + return 0, err + } + + if tagCount != 0 { + return 0, errUnsupportedTaggedFields + } + + return 0, nil +} + // collections func (rd *realDecoder) getBytes() ([]byte, error) { diff --git a/response_header.go b/response_header.go index 7a7591851..3b1cad618 100644 --- a/response_header.go +++ b/response_header.go @@ -10,7 +10,7 @@ type responseHeader struct { correlationID int32 } -func (r *responseHeader) decode(pd packetDecoder) (err error) { +func (r *responseHeader) decode(pd packetDecoder, version int16) (err error) { r.length, err = pd.getInt32() if err != nil { return err @@ -20,5 +20,12 @@ func (r *responseHeader) decode(pd packetDecoder) (err error) { } r.correlationID, err = pd.getInt32() + + if version >= 1 { + if _, err := pd.getEmptyTaggedFields(); err != nil { + return err + } + } + return err } diff --git a/sasl_authenticate_response.go b/sasl_authenticate_response.go index a91442cd4..3ef57b5af 100644 --- a/sasl_authenticate_response.go +++ b/sasl_authenticate_response.go @@ -40,7 +40,7 @@ func (r *SaslAuthenticateResponse) version() int16 { } func (r *SaslAuthenticateResponse) headerVersion() int16 { - return 1 + return 0 } func (r *SaslAuthenticateResponse) requiredVersion() KafkaVersion { diff --git a/sasl_handshake_response.go b/sasl_handshake_response.go index 6b99843bb..69dfc3178 100644 --- a/sasl_handshake_response.go +++ b/sasl_handshake_response.go @@ -34,7 +34,7 @@ func (r *SaslHandshakeResponse) version() int16 { } func (r *SaslHandshakeResponse) headerVersion() int16 { - return 1 + return 0 } func (r *SaslHandshakeResponse) requiredVersion() KafkaVersion { diff --git a/sync_group_response.go b/sync_group_response.go index 3a6f8a7bc..af019c42f 100644 --- a/sync_group_response.go +++ b/sync_group_response.go @@ -37,7 +37,7 @@ func (r *SyncGroupResponse) version() int16 { } func (r *SyncGroupResponse) headerVersion() int16 { - return 1 + return 0 } func (r *SyncGroupResponse) requiredVersion() KafkaVersion { diff --git a/txn_offset_commit_response.go b/txn_offset_commit_response.go index 25f5475d2..94d8029da 100644 --- a/txn_offset_commit_response.go +++ b/txn_offset_commit_response.go @@ -79,7 +79,7 @@ func (a *TxnOffsetCommitResponse) version() int16 { } func (a *TxnOffsetCommitResponse) headerVersion() int16 { - return 1 + return 0 } func (a *TxnOffsetCommitResponse) requiredVersion() KafkaVersion { From 314db3f228f2588e997761c992d52f27c8027287 Mon Sep 17 00:00:00 2001 From: Dirk Wilden Date: Fri, 14 Feb 2020 13:57:44 +0100 Subject: [PATCH 11/20] add ListPartitionReassignmentsResponse --- list_partition_reassignments_response.go | 171 ++++++++++++++++++ list_partition_reassignments_response_test.go | 32 ++++ 2 files changed, 203 insertions(+) create mode 100644 list_partition_reassignments_response.go create mode 100644 list_partition_reassignments_response_test.go diff --git a/list_partition_reassignments_response.go b/list_partition_reassignments_response.go new file mode 100644 index 000000000..bab634a26 --- /dev/null +++ b/list_partition_reassignments_response.go @@ -0,0 +1,171 @@ +package sarama + +type listPartitionReassignmentsResponseBlock struct { + replicas []int32 + addingReplicas []int32 + removingReplicas []int32 +} + +func (b *listPartitionReassignmentsResponseBlock) encode(pe packetEncoder) error { + + if err := pe.putCompactInt32Array(b.replicas); err != nil { + return err + } + if err := pe.putCompactInt32Array(b.addingReplicas); err != nil { + return err + } + if err := pe.putCompactInt32Array(b.removingReplicas); err != nil { + return err + } + + pe.putEmptyTaggedFieldArray() + + return nil +} + +func (b *listPartitionReassignmentsResponseBlock) decode(pd packetDecoder) (err error) { + + if b.replicas, err = pd.getCompactInt32Array(); err != nil { + return err + } + + if b.addingReplicas, err = pd.getCompactInt32Array(); err != nil { + return err + } + + if b.removingReplicas, err = pd.getCompactInt32Array(); err != nil { + return err + } + + if _, err := pd.getEmptyTaggedFieldArray(); err != nil { + return err + } + + return err +} + +type ListPartitionReassignmentsResponse struct { + Version int16 + ThrottleTimeMs int32 + ErrorCode KError + ErrorMessage *string + blocks map[string]map[int32]*listPartitionReassignmentsResponseBlock +} + +func (r *ListPartitionReassignmentsResponse) AddBlock(topic string, partition int32, replicas, addingReplicas, removingReplicas []int32) { + if r.blocks == nil { + r.blocks = make(map[string]map[int32]*listPartitionReassignmentsResponseBlock) + } + partitions := r.blocks[topic] + if partitions == nil { + partitions = make(map[int32]*listPartitionReassignmentsResponseBlock) + r.blocks[topic] = partitions + } + + partitions[partition] = &listPartitionReassignmentsResponseBlock{replicas: replicas, addingReplicas: addingReplicas, removingReplicas: removingReplicas} +} + +func (r *ListPartitionReassignmentsResponse) encode(pe packetEncoder) error { + pe.putInt32(r.ThrottleTimeMs) + pe.putInt16(int16(r.ErrorCode)) + if err := pe.putNullableCompactString(r.ErrorMessage); err != nil { + return err + } + + pe.putCompactArrayLength(len(r.blocks)) + for topic, partitions := range r.blocks { + if err := pe.putCompactString(topic); err != nil { + return err + } + pe.putCompactArrayLength(len(partitions)) + for partition, block := range partitions { + pe.putInt32(partition) + + if err := block.encode(pe); err != nil { + return err + } + } + pe.putEmptyTaggedFieldArray() + } + + pe.putEmptyTaggedFieldArray() + + return nil +} + +func (r *ListPartitionReassignmentsResponse) decode(pd packetDecoder, version int16) (err error) { + r.Version = version + + if r.ThrottleTimeMs, err = pd.getInt32(); err != nil { + return err + } + + kerr, err := pd.getInt16() + if err != nil { + return err + } + + r.ErrorCode = KError(kerr) + + if r.ErrorMessage, err = pd.getCompactNullableString(); err != nil { + return err + } + + numTopics, err := pd.getCompactArrayLength() + if err != nil || numTopics == 0 { + return err + } + + r.blocks = make(map[string]map[int32]*listPartitionReassignmentsResponseBlock, numTopics) + for i := 0; i < numTopics; i++ { + topic, err := pd.getCompactString() + if err != nil { + return err + } + + ongoingPartitionReassignments, err := pd.getCompactArrayLength() + if err != nil { + return err + } + + r.blocks[topic] = make(map[int32]*listPartitionReassignmentsResponseBlock, ongoingPartitionReassignments) + + for j := 0; j < ongoingPartitionReassignments; j++ { + partition, err := pd.getInt32() + if err != nil { + return err + } + + block := &listPartitionReassignmentsResponseBlock{} + if err := block.decode(pd); err != nil { + return err + } + r.blocks[topic][partition] = block + } + + if _, err := pd.getEmptyTaggedFieldArray(); err != nil { + return err + } + } + if _, err := pd.getEmptyTaggedFieldArray(); err != nil { + return err + } + + return nil +} + +func (r *ListPartitionReassignmentsResponse) key() int16 { + return 46 +} + +func (r *ListPartitionReassignmentsResponse) version() int16 { + return r.Version +} + +func (r *ListPartitionReassignmentsResponse) headerVersion() int16 { + return 1 +} + +func (r *ListPartitionReassignmentsResponse) requiredVersion() KafkaVersion { + return V2_4_0_0 +} diff --git a/list_partition_reassignments_response_test.go b/list_partition_reassignments_response_test.go new file mode 100644 index 000000000..ba6ca5c5b --- /dev/null +++ b/list_partition_reassignments_response_test.go @@ -0,0 +1,32 @@ +package sarama + +import "testing" + +var ( + listPartitionReassignmentsResponse = []byte{ + 0, 0, 39, 16, // ThrottleTimeMs 10000 + 0, 0, // errorcode + 0, // null string + 2, // block array length 1 + 6, 116, 111, 112, 105, 99, // topic name "topic" + 2, // partition array length 1 + 0, 0, 0, 1, // partitionId + 3, 0, 0, 3, 232, 0, 0, 3, 233, // replicas [1000, 1001] + 3, 0, 0, 3, 234, 0, 0, 3, 235, // addingReplicas [1002, 1003] + 3, 0, 0, 3, 236, 0, 0, 3, 237, // addingReplicas [1004, 1005] + 0, 0, 0, // empty tagged fields + } +) + +func TestListPartitionReassignmentResponse(t *testing.T) { + var response *ListPartitionReassignmentsResponse + + response = &ListPartitionReassignmentsResponse{ + ThrottleTimeMs: int32(10000), + Version: int16(0), + } + + response.AddBlock("topic", 1, []int32{1000, 1001}, []int32{1002, 1003}, []int32{1004, 1005}) + + testResponse(t, "one topic", response, listPartitionReassignmentsResponse) +} From 1cf58afcfb6c3600b0a602fe49247d317eba2fd5 Mon Sep 17 00:00:00 2001 From: Dirk Wilden Date: Fri, 14 Feb 2020 13:58:17 +0100 Subject: [PATCH 12/20] add helper for putEmptyTaggedFieldArray --- alter_partition_reassignments_request.go | 48 +++++-------- alter_partition_reassignments_request_test.go | 53 -------------- alter_partition_reassignments_response.go | 69 ++++++++----------- broker.go | 13 ++++ list_partition_reassignments_request.go | 14 ++-- packet_decoder.go | 3 +- packet_encoder.go | 1 + prep_encoder.go | 4 ++ real_decoder.go | 24 ++++++- real_encoder.go | 4 ++ response_header.go | 2 +- response_header_test.go | 22 +++++- 12 files changed, 116 insertions(+), 141 deletions(-) diff --git a/alter_partition_reassignments_request.go b/alter_partition_reassignments_request.go index fbd7b7c0f..8b537d58e 100644 --- a/alter_partition_reassignments_request.go +++ b/alter_partition_reassignments_request.go @@ -4,28 +4,20 @@ type alterPartitionReassignmentsBlock struct { replicas []int32 } -func (b *alterPartitionReassignmentsBlock) encode(pe packetEncoder, version int16) error { - pe.putCompactInt32Array(b.replicas) - // tagged field - pe.putInt8(0) - return nil -} +func (b *alterPartitionReassignmentsBlock) encode(pe packetEncoder) error { -func (b *alterPartitionReassignmentsBlock) decode(pd packetDecoder, version int16) (err error) { - - replicaCount, err := pd.getCompactArrayLength() - if err != nil { + if err := pe.putCompactInt32Array(b.replicas); err != nil { return err } - b.replicas = make([]int32, replicaCount) + pe.putEmptyTaggedFieldArray() + return nil +} - for i := 0; i < replicaCount; i++ { - if replica, err := pd.getInt32(); err != nil { - return err - } else { - b.replicas[i] = replica - } +func (b *alterPartitionReassignmentsBlock) decode(pd packetDecoder) (err error) { + + if b.replicas, err = pd.getCompactInt32Array(); err != nil { + return err } return nil } @@ -48,16 +40,14 @@ func (r *AlterPartitionReassignmentsRequest) encode(pe packetEncoder) error { pe.putCompactArrayLength(len(partitions)) for partition, block := range partitions { pe.putInt32(partition) - if err := block.encode(pe, r.Version); err != nil { + if err := block.encode(pe); err != nil { return err } } - //another tagged field - pe.putInt8(0) + pe.putEmptyTaggedFieldArray() } - //another tagged field - pe.putInt8(0) + pe.putEmptyTaggedFieldArray() return nil } @@ -91,28 +81,22 @@ func (r *AlterPartitionReassignmentsRequest) decode(pd packetDecoder, version in return err } block := &alterPartitionReassignmentsBlock{} - if err := block.decode(pd, r.Version); err != nil { + if err := block.decode(pd); err != nil { return err } r.blocks[topic][partition] = block - // empty tagged fields array - _, err = pd.getUVarint() - if err != nil { + if _, err := pd.getEmptyTaggedFieldArray(); err != nil { return err } } - // empty tagged fields array - _, err = pd.getUVarint() - if err != nil { + if _, err := pd.getEmptyTaggedFieldArray(); err != nil { return err } } } - // empty tagged fields array - _, err = pd.getUVarint() - if err != nil { + if _, err := pd.getEmptyTaggedFieldArray(); err != nil { return err } diff --git a/alter_partition_reassignments_request_test.go b/alter_partition_reassignments_request_test.go index f6e125e6a..04192bd98 100644 --- a/alter_partition_reassignments_request_test.go +++ b/alter_partition_reassignments_request_test.go @@ -20,44 +20,6 @@ var ( 0, 0, 3, 233, // replica 1001 0, 0, 0, // empty tagged fields } - - alterPartitionReassignmentsRequestTwoBlocks = []byte{ - 0, 0, 39, 16, // timout 10000 - 3, // 3-1=2 blocks - 6, 116, 111, 112, 105, 99, // topic name "topic" as compact string - 2, // 2-1=1 partitions - 0, 0, 0, 0, // partitionId - 3, // 3-1=2 replica array size - 0, 0, 3, 232, // replica 1000 - 0, 0, 3, 233, // replica 1001 - 0, 0, // empty tagged fields - 7, 116, 111, 112, 105, 99, 50, // topic name "topic2" as compact string - 2, // 2-1=1 partitions - 0, 0, 0, 1, // partitionId - 3, // 3-1=2 replica array size - 0, 0, 3, 233, // replica 1001 - 0, 0, 3, 234, // replica 1002 - 0, 0, // empty tagged fields - 0, // empty tagged fields - } - - alterPartitionReassignmentsRequestTwoPartitions = []byte{ - 0, 0, 39, 16, // timout 10000 - 2, // 2-1=1 block - 6, 116, 111, 112, 105, 99, // topic name "topic" as compact string - 3, // 3-1=2 partitions - 0, 0, 0, 0, // partitionId - 3, // 3-1=2 replica array size - 0, 0, 3, 232, // replica 1000 - 0, 0, 3, 233, // replica 1001 - 0, // empty tagged fields - 0, 0, 0, 1, // partitionId - 3, // 3-1=2 replica array size - 0, 0, 3, 233, // replica 1001 - 0, 0, 3, 234, // replica 1002 - 0, 0, // empty tagged fields - 0, // empty tagged fields - } ) func TestAlterPartitionReassignmentRequest(t *testing.T) { @@ -73,19 +35,4 @@ func TestAlterPartitionReassignmentRequest(t *testing.T) { request.AddBlock("topic", 0, []int32{1000, 1001}) testRequest(t, "one block", request, alterPartitionReassignmentsRequestOneBlock) - - request.AddBlock("topic2", 1, []int32{1001, 1002}) - - testRequest(t, "two blocks", request, alterPartitionReassignmentsRequestTwoBlocks) - - request = &AlterPartitionReassignmentsRequest{ - TimeoutMs: int32(10000), - Version: int16(0), - } - - request.AddBlock("topic", 0, []int32{1000, 1001}) - request.AddBlock("topic", 1, []int32{1001, 1002}) - - testRequest(t, "two partitions", request, alterPartitionReassignmentsRequestTwoPartitions) - } diff --git a/alter_partition_reassignments_response.go b/alter_partition_reassignments_response.go index 30538d91e..b3f9a15fe 100644 --- a/alter_partition_reassignments_response.go +++ b/alter_partition_reassignments_response.go @@ -10,19 +10,22 @@ func (b *alterPartitionReassignmentsErrorBlock) encode(pe packetEncoder) error { if err := pe.putNullableCompactString(b.errorMessage); err != nil { return err } - // tagged field - pe.putUVarint(0) + pe.putEmptyTaggedFieldArray() return nil } func (b *alterPartitionReassignmentsErrorBlock) decode(pd packetDecoder) (err error) { - errorCode, err := pd.getInt32() + errorCode, err := pd.getInt16() if err != nil { return err } b.errorCode = KError(errorCode) b.errorMessage, err = pd.getCompactNullableString() + + if _, err := pd.getEmptyTaggedFieldArray(); err != nil { + return err + } return err } @@ -67,13 +70,10 @@ func (r *AlterPartitionReassignmentsResponse) encode(pe packetEncoder) error { return err } } - // tagged field - pe.putUVarint(0) + pe.putEmptyTaggedFieldArray() } - // tagged field - pe.putUVarint(0) - + pe.putEmptyTaggedFieldArray() return nil } @@ -96,55 +96,44 @@ func (r *AlterPartitionReassignmentsResponse) decode(pd packetDecoder, version i } numTopics, err := pd.getCompactArrayLength() - if err != nil || numTopics == 0 { + if err != nil { return err } - r.Errors = make(map[string]map[int32]*alterPartitionReassignmentsErrorBlock, numTopics) - for i := 0; i < int(numTopics); i++ { - name, err := pd.getCompactString() - if err != nil { - return err - } - - ongoingPartitionReassignments, err := pd.getCompactArrayLength() - if err != nil { - return err - } - - r.Errors[name] = make(map[int32]*alterPartitionReassignmentsErrorBlock, ongoingPartitionReassignments) - - for j := 0; j < ongoingPartitionReassignments; j++ { - partition, err := pd.getInt32() - if err != nil { - return err - } - errorCode, err := pd.getInt16() + if numTopics > 0 { + r.Errors = make(map[string]map[int32]*alterPartitionReassignmentsErrorBlock, numTopics) + for i := 0; i < numTopics; i++ { + topic, err := pd.getCompactString() if err != nil { return err } - errorMessage, err := pd.getCompactNullableString() + + ongoingPartitionReassignments, err := pd.getCompactArrayLength() if err != nil { return err } - if errorCode != 0 { - if errorMessage == nil { - errorMessage = new(string) + r.Errors[topic] = make(map[int32]*alterPartitionReassignmentsErrorBlock, ongoingPartitionReassignments) + + for j := 0; j < ongoingPartitionReassignments; j++ { + partition, err := pd.getInt32() + if err != nil { + return err + } + block := &alterPartitionReassignmentsErrorBlock{} + if err := block.decode(pd); err != nil { + return err } - r.AddError(name, partition, KError(errorCode), errorMessage) + r.Errors[topic][partition] = block } - - if _, err = pd.getEmptyTaggedFields(); err != nil { + if _, err = pd.getEmptyTaggedFieldArray(); err != nil { return err } } - if _, err = pd.getEmptyTaggedFields(); err != nil { - return err - } } - if _, err = pd.getEmptyTaggedFields(); err != nil { + + if _, err = pd.getEmptyTaggedFieldArray(); err != nil { return err } diff --git a/broker.go b/broker.go index 723c4c920..5c6e31618 100644 --- a/broker.go +++ b/broker.go @@ -527,6 +527,19 @@ func (b *Broker) AlterPartitionReassignments(request *AlterPartitionReassignment return response, nil } +//ListPartitionReassignments sends a list partition reassignments request and +//returns list partition reassignments response +func (b *Broker) ListPartitionReassignments(request *ListPartitionReassignmentsRequest) (*ListPartitionReassignmentsResponse, error) { + response := new(ListPartitionReassignmentsResponse) + + err := b.sendAndReceive(request, response) + if err != nil { + return nil, err + } + + return response, nil +} + //DeleteRecords send a request to delete records and return delete record //response or error func (b *Broker) DeleteRecords(request *DeleteRecordsRequest) (*DeleteRecordsResponse, error) { diff --git a/list_partition_reassignments_request.go b/list_partition_reassignments_request.go index 7363f3ed6..c1ffa9ba0 100644 --- a/list_partition_reassignments_request.go +++ b/list_partition_reassignments_request.go @@ -20,12 +20,10 @@ func (r *ListPartitionReassignmentsRequest) encode(pe packetEncoder) error { return err } - //another tagged field - pe.putInt8(0) + pe.putEmptyTaggedFieldArray() } - //another tagged field - pe.putInt8(0) + pe.putEmptyTaggedFieldArray() return nil } @@ -60,17 +58,13 @@ func (r *ListPartitionReassignmentsRequest) decode(pd packetDecoder, version int } r.blocks[topic][j] = partition } - // empty tagged fields array - _, err = pd.getUVarint() - if err != nil { + if _, err := pd.getEmptyTaggedFieldArray(); err != nil { return err } } } - // empty tagged fields array - _, err = pd.getUVarint() - if err != nil { + if _, err := pd.getEmptyTaggedFieldArray(); err != nil { return err } diff --git a/packet_decoder.go b/packet_decoder.go index 5e0b0a07f..ed00ba350 100644 --- a/packet_decoder.go +++ b/packet_decoder.go @@ -14,7 +14,7 @@ type packetDecoder interface { getArrayLength() (int, error) getCompactArrayLength() (int, error) getBool() (bool, error) - getEmptyTaggedFields() (int, error) + getEmptyTaggedFieldArray() (int, error) // Collections getBytes() ([]byte, error) @@ -24,6 +24,7 @@ type packetDecoder interface { getNullableString() (*string, error) getCompactString() (string, error) getCompactNullableString() (*string, error) + getCompactInt32Array() ([]int32, error) getInt32Array() ([]int32, error) getInt64Array() ([]int64, error) getStringArray() ([]string, error) diff --git a/packet_encoder.go b/packet_encoder.go index 78784683b..6ae9d9766 100644 --- a/packet_encoder.go +++ b/packet_encoder.go @@ -29,6 +29,7 @@ type packetEncoder interface { putCompactInt32Array(in []int32) error putInt32Array(in []int32) error putInt64Array(in []int64) error + putEmptyTaggedFieldArray() // Provide the current offset to record the batch size metric offset() int diff --git a/prep_encoder.go b/prep_encoder.go index 04660daf4..af87a233b 100644 --- a/prep_encoder.go +++ b/prep_encoder.go @@ -154,6 +154,10 @@ func (pe *prepEncoder) putInt64Array(in []int64) error { return nil } +func (pe *prepEncoder) putEmptyTaggedFieldArray() { + pe.putUVarint(0) +} + func (pe *prepEncoder) offset() int { return pe.length } diff --git a/real_decoder.go b/real_decoder.go index 4ebad2d89..0178ba569 100644 --- a/real_decoder.go +++ b/real_decoder.go @@ -131,7 +131,7 @@ func (rd *realDecoder) getBool() (bool, error) { return true, nil } -func (rd *realDecoder) getEmptyTaggedFields() (int, error) { +func (rd *realDecoder) getEmptyTaggedFieldArray() (int, error) { tagCount, err := rd.getUVarint() if err != nil { return 0, err @@ -242,6 +242,28 @@ func (rd *realDecoder) getCompactNullableString() (*string, error) { return &tmpStr, err } +func (rd *realDecoder) getCompactInt32Array() ([]int32, error) { + + n, err := rd.getUVarint() + if err != nil { + return nil, err + } + + if n == 0 { + return nil, nil + } + + arrayLength := int(n) - 1 + + ret := make([]int32, arrayLength) + + for i := range ret { + ret[i] = int32(binary.BigEndian.Uint32(rd.raw[rd.off:])) + rd.off += 4 + } + return ret, nil +} + func (rd *realDecoder) getInt32Array() ([]int32, error) { if rd.remaining() < 4 { rd.off = len(rd.raw) diff --git a/real_encoder.go b/real_encoder.go index 53b08ae9c..603d8418d 100644 --- a/real_encoder.go +++ b/real_encoder.go @@ -161,6 +161,10 @@ func (re *realEncoder) putInt64Array(in []int64) error { return nil } +func (re *realEncoder) putEmptyTaggedFieldArray() { + re.putUVarint(0) +} + func (re *realEncoder) offset() int { return re.off } diff --git a/response_header.go b/response_header.go index 3b1cad618..5dffb75be 100644 --- a/response_header.go +++ b/response_header.go @@ -22,7 +22,7 @@ func (r *responseHeader) decode(pd packetDecoder, version int16) (err error) { r.correlationID, err = pd.getInt32() if version >= 1 { - if _, err := pd.getEmptyTaggedFields(); err != nil { + if _, err := pd.getEmptyTaggedFieldArray(); err != nil { return err } } diff --git a/response_header_test.go b/response_header_test.go index 8f9fdb80c..31c35ae6a 100644 --- a/response_header_test.go +++ b/response_header_test.go @@ -3,15 +3,31 @@ package sarama import "testing" var ( - responseHeaderBytes = []byte{ + responseHeaderBytesV0 = []byte{ 0x00, 0x00, 0x0f, 0x00, 0x0a, 0xbb, 0xcc, 0xff} + + responseHeaderBytesV1 = []byte{ + 0x00, 0x00, 0x0f, 0x00, + 0x0a, 0xbb, 0xcc, 0xff, 0x00} ) -func TestResponseHeader(t *testing.T) { +func TestResponseHeaderV0(t *testing.T) { + header := responseHeader{} + + testVersionDecodable(t, "response header", &header, responseHeaderBytesV0, 0) + if header.length != 0xf00 { + t.Error("Decoding header length failed, got", header.length) + } + if header.correlationID != 0x0abbccff { + t.Error("Decoding header correlation id failed, got", header.correlationID) + } +} + +func TestResponseHeaderV1(t *testing.T) { header := responseHeader{} - testDecodable(t, "response header", &header, responseHeaderBytes) + testVersionDecodable(t, "response header", &header, responseHeaderBytesV1, 1) if header.length != 0xf00 { t.Error("Decoding header length failed, got", header.length) } From 9ef5c3b756ebab54047199b2effc8b2936657839 Mon Sep 17 00:00:00 2001 From: Dirk Wilden Date: Fri, 14 Feb 2020 14:08:05 +0100 Subject: [PATCH 13/20] fix problem when no responseBody is provided --- broker.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/broker.go b/broker.go index 5c6e31618..a27a5f6f9 100644 --- a/broker.go +++ b/broker.go @@ -765,7 +765,13 @@ func (b *Broker) send(rb protocolBody, promiseResponse bool, responseHeaderVersi } func (b *Broker) sendAndReceive(req protocolBody, res protocolBody) error { - promise, err := b.send(req, res != nil, res.headerVersion()) + + responseHeaderVersion := int16(-1) + if res != nil { + responseHeaderVersion = res.headerVersion() + } + + promise, err := b.send(req, res != nil, responseHeaderVersion) if err != nil { return err } From 023606afe3d65e53de820c4511111b5cce77421d Mon Sep 17 00:00:00 2001 From: Dirk Wilden Date: Sun, 16 Feb 2020 13:10:13 +0100 Subject: [PATCH 14/20] make sure mockbroker can handle different reponse header versions --- async_producer_test.go | 2 +- broker.go | 4 +-- broker_test.go | 4 +++ encoder_decoder.go | 5 ++++ mockbroker.go | 35 ++++++++++++++++++------- mockresponses.go | 58 +++++++++++++++++++++--------------------- offset_manager_test.go | 6 ++--- 7 files changed, 70 insertions(+), 44 deletions(-) diff --git a/async_producer_test.go b/async_producer_test.go index 3de308680..d0f012d24 100644 --- a/async_producer_test.go +++ b/async_producer_test.go @@ -988,7 +988,7 @@ func TestAsyncProducerIdempotentRetryCheckBatch(t *testing.T) { lastBatchFirstSeq := -1 lastBatchSize := -1 lastSequenceWrittenToDisk := -1 - handlerFailBeforeWrite := func(req *request) (res encoder) { + handlerFailBeforeWrite := func(req *request) (res encoderWithHeader) { switch req.body.key() { case 3: return metadataResponse diff --git a/broker.go b/broker.go index a27a5f6f9..a71b274ad 100644 --- a/broker.go +++ b/broker.go @@ -1225,7 +1225,7 @@ func (b *Broker) receiveSaslAuthenticateResponse(correlationID int32) ([]byte, e } header := responseHeader{} - err = versionedDecode(buf, &header, 1) + err = versionedDecode(buf, &header, 0) if err != nil { return nil, err } @@ -1314,7 +1314,7 @@ func (b *Broker) receiveSASLServerResponse(res *SaslAuthenticateResponse, correl } header := responseHeader{} - err = versionedDecode(buf, &header, 1) + err = versionedDecode(buf, &header, 0) if err != nil { return bytesRead, err } diff --git a/broker_test.go b/broker_test.go index 387dae952..e2b17462c 100644 --- a/broker_test.go +++ b/broker_test.go @@ -42,6 +42,10 @@ func (m mockEncoder) encode(pe packetEncoder) error { return pe.putRawBytes(m.bytes) } +func (m mockEncoder) headerVersion() int16 { + return 0 +} + type brokerMetrics struct { bytesRead int bytesWritten int diff --git a/encoder_decoder.go b/encoder_decoder.go index 7ce3bc0f6..025bad61f 100644 --- a/encoder_decoder.go +++ b/encoder_decoder.go @@ -12,6 +12,11 @@ type encoder interface { encode(pe packetEncoder) error } +type encoderWithHeader interface { + encoder + headerVersion() int16 +} + // Encode takes an Encoder and turns it into bytes while potentially recording metrics. func encode(e encoder, metricRegistry metrics.Registry) ([]byte, error) { if e == nil { diff --git a/mockbroker.go b/mockbroker.go index 56e3436ef..b72b1ca2f 100644 --- a/mockbroker.go +++ b/mockbroker.go @@ -20,7 +20,7 @@ const ( type GSSApiHandlerFunc func([]byte) []byte -type requestHandlerFunc func(req *request) (res encoder) +type requestHandlerFunc func(req *request) (res encoderWithHeader) // RequestNotifierFunc is invoked when a mock broker processes a request successfully // and will provides the number of bytes read and written. @@ -55,7 +55,7 @@ type MockBroker struct { port int32 closing chan none stopper chan none - expectations chan encoder + expectations chan encoderWithHeader listener net.Listener t TestReporter latency time.Duration @@ -83,7 +83,7 @@ func (b *MockBroker) SetLatency(latency time.Duration) { // and uses the found MockResponse instance to generate an appropriate reply. // If the request type is not found in the map then nothing is sent. func (b *MockBroker) SetHandlerByMap(handlerMap map[string]MockResponse) { - b.setHandler(func(req *request) (res encoder) { + b.setHandler(func(req *request) (res encoderWithHeader) { reqTypeName := reflect.TypeOf(req.body).Elem().Name() mockResponse := handlerMap[reqTypeName] if mockResponse == nil { @@ -231,7 +231,6 @@ func (b *MockBroker) handleRequests(conn io.ReadWriteCloser, idx int, wg *sync.W } }() - resHeader := make([]byte, 8) var bytesWritten int var bytesRead int for { @@ -281,8 +280,7 @@ func (b *MockBroker) handleRequests(conn io.ReadWriteCloser, idx int, wg *sync.W continue } - binary.BigEndian.PutUint32(resHeader, uint32(len(encodedRes)+4)) - binary.BigEndian.PutUint32(resHeader[4:], uint32(req.correlationID)) + resHeader := b.encodeHeader(res.headerVersion(), req.correlationID, uint32(len(encodedRes))) if _, err = conn.Write(resHeader); err != nil { b.serverError(err) break @@ -318,7 +316,26 @@ func (b *MockBroker) handleRequests(conn io.ReadWriteCloser, idx int, wg *sync.W Logger.Printf("*** mockbroker/%d/%d: connection closed, err=%v", b.BrokerID(), idx, err) } -func (b *MockBroker) defaultRequestHandler(req *request) (res encoder) { +func (b *MockBroker) encodeHeader(headerVersion int16, correlationId int32, payloadLength uint32) []byte { + + headerLength := uint32(8) + + if headerVersion >= 1 { + headerLength = 9 + } + + resHeader := make([]byte, headerLength) + binary.BigEndian.PutUint32(resHeader, payloadLength+headerLength-4) + binary.BigEndian.PutUint32(resHeader[4:], uint32(correlationId)) + + if headerVersion >= 1 { + binary.PutUvarint(resHeader[8:], 0) + } + + return resHeader +} + +func (b *MockBroker) defaultRequestHandler(req *request) (res encoderWithHeader) { select { case res, ok := <-b.expectations: if !ok { @@ -373,7 +390,7 @@ func NewMockBrokerListener(t TestReporter, brokerID int32, listener net.Listener stopper: make(chan none), t: t, brokerID: brokerID, - expectations: make(chan encoder, 512), + expectations: make(chan encoderWithHeader, 512), listener: listener, } broker.handler = broker.defaultRequestHandler @@ -394,6 +411,6 @@ func NewMockBrokerListener(t TestReporter, brokerID int32, listener net.Listener return broker } -func (b *MockBroker) Returns(e encoder) { +func (b *MockBroker) Returns(e encoderWithHeader) { b.expectations <- e } diff --git a/mockresponses.go b/mockresponses.go index 8c8cd055e..2f9279f18 100644 --- a/mockresponses.go +++ b/mockresponses.go @@ -18,20 +18,20 @@ type TestReporter interface { // allows generating a response based on a request body. MockResponses are used // to program behavior of MockBroker in tests. type MockResponse interface { - For(reqBody versionedDecoder) (res encoder) + For(reqBody versionedDecoder) (res encoderWithHeader) } // MockWrapper is a mock response builder that returns a particular concrete // response regardless of the actual request passed to the `For` method. type MockWrapper struct { - res encoder + res encoderWithHeader } -func (mw *MockWrapper) For(reqBody versionedDecoder) (res encoder) { +func (mw *MockWrapper) For(reqBody versionedDecoder) (res encoderWithHeader) { return mw.res } -func NewMockWrapper(res encoder) *MockWrapper { +func NewMockWrapper(res encoderWithHeader) *MockWrapper { return &MockWrapper{res: res} } @@ -50,7 +50,7 @@ func NewMockSequence(responses ...interface{}) *MockSequence { switch res := res.(type) { case MockResponse: ms.responses[i] = res - case encoder: + case encoderWithHeader: ms.responses[i] = NewMockWrapper(res) default: panic(fmt.Sprintf("Unexpected response type: %T", res)) @@ -59,7 +59,7 @@ func NewMockSequence(responses ...interface{}) *MockSequence { return ms } -func (mc *MockSequence) For(reqBody versionedDecoder) (res encoder) { +func (mc *MockSequence) For(reqBody versionedDecoder) (res encoderWithHeader) { res = mc.responses[0].For(reqBody) if len(mc.responses) > 1 { mc.responses = mc.responses[1:] @@ -79,7 +79,7 @@ func NewMockListGroupsResponse(t TestReporter) *MockListGroupsResponse { } } -func (m *MockListGroupsResponse) For(reqBody versionedDecoder) encoder { +func (m *MockListGroupsResponse) For(reqBody versionedDecoder) encoderWithHeader { request := reqBody.(*ListGroupsRequest) _ = request response := &ListGroupsResponse{ @@ -110,7 +110,7 @@ func (m *MockDescribeGroupsResponse) AddGroupDescription(groupID string, descrip return m } -func (m *MockDescribeGroupsResponse) For(reqBody versionedDecoder) encoder { +func (m *MockDescribeGroupsResponse) For(reqBody versionedDecoder) encoderWithHeader { request := reqBody.(*DescribeGroupsRequest) response := &DescribeGroupsResponse{} @@ -166,7 +166,7 @@ func (mmr *MockMetadataResponse) SetController(brokerID int32) *MockMetadataResp return mmr } -func (mmr *MockMetadataResponse) For(reqBody versionedDecoder) encoder { +func (mmr *MockMetadataResponse) For(reqBody versionedDecoder) encoderWithHeader { metadataRequest := reqBody.(*MetadataRequest) metadataResponse := &MetadataResponse{ Version: metadataRequest.version(), @@ -233,7 +233,7 @@ func (mor *MockOffsetResponse) SetOffset(topic string, partition int32, time, of return mor } -func (mor *MockOffsetResponse) For(reqBody versionedDecoder) encoder { +func (mor *MockOffsetResponse) For(reqBody versionedDecoder) encoderWithHeader { offsetRequest := reqBody.(*OffsetRequest) offsetResponse := &OffsetResponse{Version: mor.version} for topic, partitions := range offsetRequest.blocks { @@ -309,7 +309,7 @@ func (mfr *MockFetchResponse) SetHighWaterMark(topic string, partition int32, of return mfr } -func (mfr *MockFetchResponse) For(reqBody versionedDecoder) encoder { +func (mfr *MockFetchResponse) For(reqBody versionedDecoder) encoderWithHeader { fetchRequest := reqBody.(*FetchRequest) res := &FetchResponse{ Version: mfr.version, @@ -393,7 +393,7 @@ func (mr *MockConsumerMetadataResponse) SetError(group string, kerror KError) *M return mr } -func (mr *MockConsumerMetadataResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockConsumerMetadataResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*ConsumerMetadataRequest) group := req.ConsumerGroup res := &ConsumerMetadataResponse{} @@ -442,7 +442,7 @@ func (mr *MockFindCoordinatorResponse) SetError(coordinatorType CoordinatorType, return mr } -func (mr *MockFindCoordinatorResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockFindCoordinatorResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*FindCoordinatorRequest) res := &FindCoordinatorResponse{} var v interface{} @@ -489,7 +489,7 @@ func (mr *MockOffsetCommitResponse) SetError(group, topic string, partition int3 return mr } -func (mr *MockOffsetCommitResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockOffsetCommitResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*OffsetCommitRequest) group := req.ConsumerGroup res := &OffsetCommitResponse{} @@ -546,7 +546,7 @@ func (mr *MockProduceResponse) SetError(topic string, partition int32, kerror KE return mr } -func (mr *MockProduceResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockProduceResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*ProduceRequest) res := &ProduceResponse{ Version: mr.version, @@ -605,7 +605,7 @@ func (mr *MockOffsetFetchResponse) SetError(kerror KError) *MockOffsetFetchRespo return mr } -func (mr *MockOffsetFetchResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockOffsetFetchResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*OffsetFetchRequest) group := req.ConsumerGroup res := &OffsetFetchResponse{Version: req.Version} @@ -630,7 +630,7 @@ func NewMockCreateTopicsResponse(t TestReporter) *MockCreateTopicsResponse { return &MockCreateTopicsResponse{t: t} } -func (mr *MockCreateTopicsResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockCreateTopicsResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*CreateTopicsRequest) res := &CreateTopicsResponse{ Version: req.Version, @@ -659,7 +659,7 @@ func NewMockDeleteTopicsResponse(t TestReporter) *MockDeleteTopicsResponse { return &MockDeleteTopicsResponse{t: t} } -func (mr *MockDeleteTopicsResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockDeleteTopicsResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*DeleteTopicsRequest) res := &DeleteTopicsResponse{} res.TopicErrorCodes = make(map[string]KError) @@ -679,7 +679,7 @@ func NewMockCreatePartitionsResponse(t TestReporter) *MockCreatePartitionsRespon return &MockCreatePartitionsResponse{t: t} } -func (mr *MockCreatePartitionsResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockCreatePartitionsResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*CreatePartitionsRequest) res := &CreatePartitionsResponse{} res.TopicPartitionErrors = make(map[string]*TopicPartitionError) @@ -706,7 +706,7 @@ func NewMockAlterPartitionReassignmentsResponse(t TestReporter) *MockAlterPartit return &MockAlterPartitionReassignmentsResponse{t: t} } -func (mr *MockAlterPartitionReassignmentsResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockAlterPartitionReassignmentsResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*AlterPartitionReassignmentsRequest) _ = req res := &AlterPartitionReassignmentsResponse{} @@ -721,7 +721,7 @@ func NewMockDeleteRecordsResponse(t TestReporter) *MockDeleteRecordsResponse { return &MockDeleteRecordsResponse{t: t} } -func (mr *MockDeleteRecordsResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockDeleteRecordsResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*DeleteRecordsRequest) res := &DeleteRecordsResponse{} res.Topics = make(map[string]*DeleteRecordsResponseTopic) @@ -744,7 +744,7 @@ func NewMockDescribeConfigsResponse(t TestReporter) *MockDescribeConfigsResponse return &MockDescribeConfigsResponse{t: t} } -func (mr *MockDescribeConfigsResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockDescribeConfigsResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*DescribeConfigsRequest) res := &DescribeConfigsResponse{ Version: req.Version, @@ -835,7 +835,7 @@ func NewMockAlterConfigsResponse(t TestReporter) *MockAlterConfigsResponse { return &MockAlterConfigsResponse{t: t} } -func (mr *MockAlterConfigsResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockAlterConfigsResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*AlterConfigsRequest) res := &AlterConfigsResponse{} @@ -856,7 +856,7 @@ func NewMockCreateAclsResponse(t TestReporter) *MockCreateAclsResponse { return &MockCreateAclsResponse{t: t} } -func (mr *MockCreateAclsResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockCreateAclsResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*CreateAclsRequest) res := &CreateAclsResponse{} @@ -874,7 +874,7 @@ func NewMockListAclsResponse(t TestReporter) *MockListAclsResponse { return &MockListAclsResponse{t: t} } -func (mr *MockListAclsResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockListAclsResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*DescribeAclsRequest) res := &DescribeAclsResponse{} res.Err = ErrNoError @@ -916,7 +916,7 @@ func NewMockSaslAuthenticateResponse(t TestReporter) *MockSaslAuthenticateRespon return &MockSaslAuthenticateResponse{t: t} } -func (msar *MockSaslAuthenticateResponse) For(reqBody versionedDecoder) encoder { +func (msar *MockSaslAuthenticateResponse) For(reqBody versionedDecoder) encoderWithHeader { res := &SaslAuthenticateResponse{} res.Err = msar.kerror res.SaslAuthBytes = msar.saslAuthBytes @@ -947,7 +947,7 @@ func NewMockSaslHandshakeResponse(t TestReporter) *MockSaslHandshakeResponse { return &MockSaslHandshakeResponse{t: t} } -func (mshr *MockSaslHandshakeResponse) For(reqBody versionedDecoder) encoder { +func (mshr *MockSaslHandshakeResponse) For(reqBody versionedDecoder) encoderWithHeader { res := &SaslHandshakeResponse{} res.Err = mshr.kerror res.EnabledMechanisms = mshr.enabledMechanisms @@ -968,7 +968,7 @@ func NewMockDeleteAclsResponse(t TestReporter) *MockDeleteAclsResponse { return &MockDeleteAclsResponse{t: t} } -func (mr *MockDeleteAclsResponse) For(reqBody versionedDecoder) encoder { +func (mr *MockDeleteAclsResponse) For(reqBody versionedDecoder) encoderWithHeader { req := reqBody.(*DeleteAclsRequest) res := &DeleteAclsResponse{} @@ -994,7 +994,7 @@ func (m *MockDeleteGroupsResponse) SetDeletedGroups(groups []string) *MockDelete return m } -func (m *MockDeleteGroupsResponse) For(reqBody versionedDecoder) encoder { +func (m *MockDeleteGroupsResponse) For(reqBody versionedDecoder) encoderWithHeader { resp := &DeleteGroupsResponse{ GroupErrorCodes: map[string]KError{}, } diff --git a/offset_manager_test.go b/offset_manager_test.go index eca926d52..f1baa9cdb 100644 --- a/offset_manager_test.go +++ b/offset_manager_test.go @@ -134,7 +134,7 @@ func TestNewOffsetManagerOffsetsAutoCommit(t *testing.T) { ocResponse := new(OffsetCommitResponse) ocResponse.AddError("my_topic", 0, ErrNoError) - handler := func(req *request) (res encoder) { + handler := func(req *request) (res encoderWithHeader) { close(called) return ocResponse } @@ -329,7 +329,7 @@ func TestPartitionOffsetManagerResetOffsetWithRetention(t *testing.T) { ocResponse := new(OffsetCommitResponse) ocResponse.AddError("my_topic", 0, ErrNoError) - handler := func(req *request) (res encoder) { + handler := func(req *request) (res encoderWithHeader) { if req.body.version() != 2 { t.Errorf("Expected to be using version 2. Actual: %v", req.body.version()) } @@ -390,7 +390,7 @@ func TestPartitionOffsetManagerMarkOffsetWithRetention(t *testing.T) { ocResponse := new(OffsetCommitResponse) ocResponse.AddError("my_topic", 0, ErrNoError) - handler := func(req *request) (res encoder) { + handler := func(req *request) (res encoderWithHeader) { if req.body.version() != 2 { t.Errorf("Expected to be using version 2. Actual: %v", req.body.version()) } From aaf9f169a690a8cbe6df87a1148a2bf1e302f8b0 Mon Sep 17 00:00:00 2001 From: Dirk Wilden Date: Sun, 16 Feb 2020 13:46:19 +0100 Subject: [PATCH 15/20] make sure partition reassignment can be aborted --- alter_partition_reassignments_request.go | 2 +- alter_partition_reassignments_request_test.go | 18 ++++++++++++++++++ packet_encoder.go | 1 + prep_encoder.go | 18 ++++++++++++++++++ real_encoder.go | 17 +++++++++++++++++ 5 files changed, 55 insertions(+), 1 deletion(-) diff --git a/alter_partition_reassignments_request.go b/alter_partition_reassignments_request.go index 8b537d58e..776436e1b 100644 --- a/alter_partition_reassignments_request.go +++ b/alter_partition_reassignments_request.go @@ -6,7 +6,7 @@ type alterPartitionReassignmentsBlock struct { func (b *alterPartitionReassignmentsBlock) encode(pe packetEncoder) error { - if err := pe.putCompactInt32Array(b.replicas); err != nil { + if err := pe.putNullableCompactInt32Array(b.replicas); err != nil { return err } diff --git a/alter_partition_reassignments_request_test.go b/alter_partition_reassignments_request_test.go index 04192bd98..8d282729d 100644 --- a/alter_partition_reassignments_request_test.go +++ b/alter_partition_reassignments_request_test.go @@ -20,6 +20,16 @@ var ( 0, 0, 3, 233, // replica 1001 0, 0, 0, // empty tagged fields } + + alterPartitionReassignmentsAbortRequest = []byte{ + 0, 0, 39, 16, // timout 10000 + 2, // 2-1=1 block + 6, 116, 111, 112, 105, 99, // topic name "topic" as compact string + 2, // 2-1=1 partitions + 0, 0, 0, 0, // partitionId + 0, // replica array is null (indicates that a pending reassignment should be aborted) + 0, 0, 0, // empty tagged fields + } ) func TestAlterPartitionReassignmentRequest(t *testing.T) { @@ -35,4 +45,12 @@ func TestAlterPartitionReassignmentRequest(t *testing.T) { request.AddBlock("topic", 0, []int32{1000, 1001}) testRequest(t, "one block", request, alterPartitionReassignmentsRequestOneBlock) + + request = &AlterPartitionReassignmentsRequest{ + TimeoutMs: int32(10000), + Version: int16(0), + } + request.AddBlock("topic", 0, nil) + + testRequest(t, "abort assignment", request, alterPartitionReassignmentsAbortRequest) } diff --git a/packet_encoder.go b/packet_encoder.go index 6ae9d9766..50c735c04 100644 --- a/packet_encoder.go +++ b/packet_encoder.go @@ -27,6 +27,7 @@ type packetEncoder interface { putNullableString(in *string) error putStringArray(in []string) error putCompactInt32Array(in []int32) error + putNullableCompactInt32Array(in []int32) error putInt32Array(in []int32) error putInt64Array(in []int64) error putEmptyTaggedFieldArray() diff --git a/prep_encoder.go b/prep_encoder.go index af87a233b..5448c814f 100644 --- a/prep_encoder.go +++ b/prep_encoder.go @@ -2,6 +2,7 @@ package sarama import ( "encoding/binary" + "errors" "fmt" "math" @@ -131,6 +132,23 @@ func (pe *prepEncoder) putStringArray(in []string) error { } func (pe *prepEncoder) putCompactInt32Array(in []int32) error { + + if in == nil { + return errors.New("expected int32 array to be non null") + } + + pe.putUVarint(uint64(len(in)) + 1) + pe.length += 4 * len(in) + return nil +} + +func (pe *prepEncoder) putNullableCompactInt32Array(in []int32) error { + + if in == nil { + pe.putUVarint(0) + return nil + } + pe.putUVarint(uint64(len(in)) + 1) pe.length += 4 * len(in) return nil diff --git a/real_encoder.go b/real_encoder.go index 603d8418d..ba073f7d3 100644 --- a/real_encoder.go +++ b/real_encoder.go @@ -2,6 +2,7 @@ package sarama import ( "encoding/binary" + "errors" "github.com/rcrowley/go-metrics" ) @@ -131,6 +132,22 @@ func (re *realEncoder) putStringArray(in []string) error { } func (re *realEncoder) putCompactInt32Array(in []int32) error { + if in == nil { + return errors.New("expected int32 array to be non null") + } + // 0 represents a null array, so +1 has to be added + re.putUVarint(uint64(len(in)) + 1) + for _, val := range in { + re.putInt32(val) + } + return nil +} + +func (re *realEncoder) putNullableCompactInt32Array(in []int32) error { + if in == nil { + re.putUVarint(0) + return nil + } // 0 represents a null array, so +1 has to be added re.putUVarint(uint64(len(in)) + 1) for _, val := range in { From c4390e150a843fc0c8c5e002ca962ee8a569e774 Mon Sep 17 00:00:00 2001 From: Dirk Wilden Date: Mon, 17 Feb 2020 13:49:42 +0100 Subject: [PATCH 16/20] add ListPartitionReassignments to admin client --- admin.go | 38 +++++++++++-- admin_test.go | 70 ++++++++++++++++++++++++ list_partition_reassignments_response.go | 32 +++++------ mockresponses.go | 23 ++++++++ 4 files changed, 142 insertions(+), 21 deletions(-) diff --git a/admin.go b/admin.go index fe440c041..2f6c8618e 100644 --- a/admin.go +++ b/admin.go @@ -42,13 +42,14 @@ type ClusterAdmin interface { // new partitions. This operation is supported by brokers with version 1.0.0 or higher. CreatePartitions(topic string, count int32, assignment [][]int32, validateOnly bool) error - // Update the configuration for the specified resources with the default options. - // This operation is supported by brokers with version 0.11.0.0 or higher. - // The resources with their configs (topic is the only resource type with configs - // that can be updated currently Updates are not transactional so they may succeed - // for some resources while fail for others. The configs for a particular resource are updated automatically. + // Alter the replica assignment for partitions. + // This operation is supported by brokers with version 2.4.0.0 or higher. AlterPartitionReassignments(topic string, assignment [][]int32) error + // Provides info on ongoing partitions replica reassignments. + // This operation is supported by brokers with version 2.4.0.0 or higher. + ListPartitionReassignments(topics string, partitions []int32) (topicStatus map[string]map[int32]*PartitionReplicaReassignmentsStatus, err error) + // Delete records whose offset is smaller than the given offset of the corresponding partition. // This operation is supported by brokers with version 0.11.0.0 or higher. DeleteRecords(topic string, partitionOffsets map[int32]int64) error @@ -500,6 +501,33 @@ func (ca *clusterAdmin) AlterPartitionReassignments(topic string, assignment [][ }) } +func (ca *clusterAdmin) ListPartitionReassignments(topic string, partitions []int32) (topicStatus map[string]map[int32]*PartitionReplicaReassignmentsStatus, err error) { + if topic == "" { + return nil, ErrInvalidTopic + } + + request := &ListPartitionReassignmentsRequest{ + TimeoutMs: int32(10000), + Version: int16(0), + } + + request.AddBlock(topic, partitions) + + b, err := ca.findAnyBroker() + if err != nil { + return nil, err + } + _ = b.Open(ca.client.Config()) + + rsp, err := b.ListPartitionReassignments(request) + + if err == nil && rsp != nil { + return rsp.TopicStatus, nil + } else { + return nil, err + } +} + func (ca *clusterAdmin) DeleteRecords(topic string, partitionOffsets map[int32]int64) error { if topic == "" { return ErrInvalidTopic diff --git a/admin_test.go b/admin_test.go index da09c7018..bae4de5f7 100644 --- a/admin_test.go +++ b/admin_test.go @@ -395,6 +395,76 @@ func TestClusterAdminAlterPartitionReassignmentsWithDiffVersion(t *testing.T) { } } +func TestClusterAdminListPartitionReassignments(t *testing.T) { + seedBroker := NewMockBroker(t, 1) + defer seedBroker.Close() + + seedBroker.SetHandlerByMap(map[string]MockResponse{ + "MetadataRequest": NewMockMetadataResponse(t). + SetController(seedBroker.BrokerID()). + SetBroker(seedBroker.Addr(), seedBroker.BrokerID()), + "ListPartitionReassignmentsRequest": NewMockListPartitionReassignmentsResponse(t), + }) + + config := NewConfig() + config.Version = V2_4_0_0 + admin, err := NewClusterAdmin([]string{seedBroker.Addr()}, config) + if err != nil { + t.Fatal(err) + } + + response, err := admin.ListPartitionReassignments("my_topic", []int32{0, 1}) + if err != nil { + t.Fatal(err) + } + + partitionStatus, ok := response["my_topic"] + if !ok { + t.Fatalf("topic missing in response") + } else { + if len(partitionStatus) != 2 { + t.Fatalf("partition missing in response") + } + } + + err = admin.Close() + if err != nil { + t.Fatal(err) + } +} + +func TestClusterAdminListPartitionReassignmentsWithDiffVersion(t *testing.T) { + seedBroker := NewMockBroker(t, 1) + defer seedBroker.Close() + + seedBroker.SetHandlerByMap(map[string]MockResponse{ + "MetadataRequest": NewMockMetadataResponse(t). + SetController(seedBroker.BrokerID()). + SetBroker(seedBroker.Addr(), seedBroker.BrokerID()), + "ListPartitionReassignmentsRequest": NewMockListPartitionReassignmentsResponse(t), + }) + + config := NewConfig() + config.Version = V2_3_0_0 + admin, err := NewClusterAdmin([]string{seedBroker.Addr()}, config) + if err != nil { + t.Fatal(err) + } + + var partitions = make([]int32, 0) + + _, err = admin.ListPartitionReassignments("my_topic", partitions) + + if !strings.ContainsAny(err.Error(), ErrUnsupportedVersion.Error()) { + t.Fatal(err) + } + + err = admin.Close() + if err != nil { + t.Fatal(err) + } +} + func TestClusterAdminDeleteRecords(t *testing.T) { topicName := "my_topic" seedBroker := NewMockBroker(t, 1) diff --git a/list_partition_reassignments_response.go b/list_partition_reassignments_response.go index bab634a26..cfc4ae5a0 100644 --- a/list_partition_reassignments_response.go +++ b/list_partition_reassignments_response.go @@ -1,12 +1,12 @@ package sarama -type listPartitionReassignmentsResponseBlock struct { +type PartitionReplicaReassignmentsStatus struct { replicas []int32 addingReplicas []int32 removingReplicas []int32 } -func (b *listPartitionReassignmentsResponseBlock) encode(pe packetEncoder) error { +func (b *PartitionReplicaReassignmentsStatus) encode(pe packetEncoder) error { if err := pe.putCompactInt32Array(b.replicas); err != nil { return err @@ -23,7 +23,7 @@ func (b *listPartitionReassignmentsResponseBlock) encode(pe packetEncoder) error return nil } -func (b *listPartitionReassignmentsResponseBlock) decode(pd packetDecoder) (err error) { +func (b *PartitionReplicaReassignmentsStatus) decode(pd packetDecoder) (err error) { if b.replicas, err = pd.getCompactInt32Array(); err != nil { return err @@ -49,20 +49,20 @@ type ListPartitionReassignmentsResponse struct { ThrottleTimeMs int32 ErrorCode KError ErrorMessage *string - blocks map[string]map[int32]*listPartitionReassignmentsResponseBlock + TopicStatus map[string]map[int32]*PartitionReplicaReassignmentsStatus } func (r *ListPartitionReassignmentsResponse) AddBlock(topic string, partition int32, replicas, addingReplicas, removingReplicas []int32) { - if r.blocks == nil { - r.blocks = make(map[string]map[int32]*listPartitionReassignmentsResponseBlock) + if r.TopicStatus == nil { + r.TopicStatus = make(map[string]map[int32]*PartitionReplicaReassignmentsStatus) } - partitions := r.blocks[topic] + partitions := r.TopicStatus[topic] if partitions == nil { - partitions = make(map[int32]*listPartitionReassignmentsResponseBlock) - r.blocks[topic] = partitions + partitions = make(map[int32]*PartitionReplicaReassignmentsStatus) + r.TopicStatus[topic] = partitions } - partitions[partition] = &listPartitionReassignmentsResponseBlock{replicas: replicas, addingReplicas: addingReplicas, removingReplicas: removingReplicas} + partitions[partition] = &PartitionReplicaReassignmentsStatus{replicas: replicas, addingReplicas: addingReplicas, removingReplicas: removingReplicas} } func (r *ListPartitionReassignmentsResponse) encode(pe packetEncoder) error { @@ -72,8 +72,8 @@ func (r *ListPartitionReassignmentsResponse) encode(pe packetEncoder) error { return err } - pe.putCompactArrayLength(len(r.blocks)) - for topic, partitions := range r.blocks { + pe.putCompactArrayLength(len(r.TopicStatus)) + for topic, partitions := range r.TopicStatus { if err := pe.putCompactString(topic); err != nil { return err } @@ -116,7 +116,7 @@ func (r *ListPartitionReassignmentsResponse) decode(pd packetDecoder, version in return err } - r.blocks = make(map[string]map[int32]*listPartitionReassignmentsResponseBlock, numTopics) + r.TopicStatus = make(map[string]map[int32]*PartitionReplicaReassignmentsStatus, numTopics) for i := 0; i < numTopics; i++ { topic, err := pd.getCompactString() if err != nil { @@ -128,7 +128,7 @@ func (r *ListPartitionReassignmentsResponse) decode(pd packetDecoder, version in return err } - r.blocks[topic] = make(map[int32]*listPartitionReassignmentsResponseBlock, ongoingPartitionReassignments) + r.TopicStatus[topic] = make(map[int32]*PartitionReplicaReassignmentsStatus, ongoingPartitionReassignments) for j := 0; j < ongoingPartitionReassignments; j++ { partition, err := pd.getInt32() @@ -136,11 +136,11 @@ func (r *ListPartitionReassignmentsResponse) decode(pd packetDecoder, version in return err } - block := &listPartitionReassignmentsResponseBlock{} + block := &PartitionReplicaReassignmentsStatus{} if err := block.decode(pd); err != nil { return err } - r.blocks[topic][partition] = block + r.TopicStatus[topic][partition] = block } if _, err := pd.getEmptyTaggedFieldArray(); err != nil { diff --git a/mockresponses.go b/mockresponses.go index 2f9279f18..0c92e6eac 100644 --- a/mockresponses.go +++ b/mockresponses.go @@ -713,6 +713,29 @@ func (mr *MockAlterPartitionReassignmentsResponse) For(reqBody versionedDecoder) return res } +type MockListPartitionReassignmentsResponse struct { + t TestReporter +} + +func NewMockListPartitionReassignmentsResponse(t TestReporter) *MockListPartitionReassignmentsResponse { + return &MockListPartitionReassignmentsResponse{t: t} +} + +func (mr *MockListPartitionReassignmentsResponse) For(reqBody versionedDecoder) encoderWithHeader { + req := reqBody.(*ListPartitionReassignmentsRequest) + _ = req + res := &ListPartitionReassignmentsResponse{} + + for topic, partitions := range req.blocks { + + for _, partition := range partitions { + res.AddBlock(topic, partition, []int32{0}, []int32{1}, []int32{2}) + } + } + + return res +} + type MockDeleteRecordsResponse struct { t TestReporter } From 30421887fb3f8ce0b507438e08d792ee5af55ffc Mon Sep 17 00:00:00 2001 From: sladkoff Date: Thu, 20 Feb 2020 08:37:52 +0100 Subject: [PATCH 17/20] Fix linter errors --- admin.go | 1 - alter_partition_reassignments_request.go | 2 -- broker.go | 2 -- list_partition_reassignments_response.go | 2 -- mockbroker.go | 1 - mockresponses.go | 1 - prep_encoder.go | 2 -- real_decoder.go | 3 +-- 8 files changed, 1 insertion(+), 13 deletions(-) diff --git a/admin.go b/admin.go index 2f6c8618e..32ba2a02b 100644 --- a/admin.go +++ b/admin.go @@ -478,7 +478,6 @@ func (ca *clusterAdmin) AlterPartitionReassignments(topic string, assignment [][ if err != nil { errs = append(errs, err) } else { - if rsp.ErrorCode > 0 { errs = append(errs, errors.New(rsp.ErrorCode.Error())) } diff --git a/alter_partition_reassignments_request.go b/alter_partition_reassignments_request.go index 776436e1b..f0a2f9dd5 100644 --- a/alter_partition_reassignments_request.go +++ b/alter_partition_reassignments_request.go @@ -5,7 +5,6 @@ type alterPartitionReassignmentsBlock struct { } func (b *alterPartitionReassignmentsBlock) encode(pe packetEncoder) error { - if err := pe.putNullableCompactInt32Array(b.replicas); err != nil { return err } @@ -15,7 +14,6 @@ func (b *alterPartitionReassignmentsBlock) encode(pe packetEncoder) error { } func (b *alterPartitionReassignmentsBlock) decode(pd packetDecoder) (err error) { - if b.replicas, err = pd.getCompactInt32Array(); err != nil { return err } diff --git a/broker.go b/broker.go index a71b274ad..4f3991af7 100644 --- a/broker.go +++ b/broker.go @@ -765,7 +765,6 @@ func (b *Broker) send(rb protocolBody, promiseResponse bool, responseHeaderVersi } func (b *Broker) sendAndReceive(req protocolBody, res protocolBody) error { - responseHeaderVersion := int16(-1) if res != nil { responseHeaderVersion = res.headerVersion() @@ -905,7 +904,6 @@ func (b *Broker) responseReceiver() { } func getHeaderLength(headerVersion int16) int8 { - if headerVersion < 1 { return 8 } else { diff --git a/list_partition_reassignments_response.go b/list_partition_reassignments_response.go index cfc4ae5a0..a5786ee7f 100644 --- a/list_partition_reassignments_response.go +++ b/list_partition_reassignments_response.go @@ -7,7 +7,6 @@ type PartitionReplicaReassignmentsStatus struct { } func (b *PartitionReplicaReassignmentsStatus) encode(pe packetEncoder) error { - if err := pe.putCompactInt32Array(b.replicas); err != nil { return err } @@ -24,7 +23,6 @@ func (b *PartitionReplicaReassignmentsStatus) encode(pe packetEncoder) error { } func (b *PartitionReplicaReassignmentsStatus) decode(pd packetDecoder) (err error) { - if b.replicas, err = pd.getCompactInt32Array(); err != nil { return err } diff --git a/mockbroker.go b/mockbroker.go index b72b1ca2f..ff5a68ae7 100644 --- a/mockbroker.go +++ b/mockbroker.go @@ -317,7 +317,6 @@ func (b *MockBroker) handleRequests(conn io.ReadWriteCloser, idx int, wg *sync.W } func (b *MockBroker) encodeHeader(headerVersion int16, correlationId int32, payloadLength uint32) []byte { - headerLength := uint32(8) if headerVersion >= 1 { diff --git a/mockresponses.go b/mockresponses.go index 0c92e6eac..e669f4803 100644 --- a/mockresponses.go +++ b/mockresponses.go @@ -727,7 +727,6 @@ func (mr *MockListPartitionReassignmentsResponse) For(reqBody versionedDecoder) res := &ListPartitionReassignmentsResponse{} for topic, partitions := range req.blocks { - for _, partition := range partitions { res.AddBlock(topic, partition, []int32{0}, []int32{1}, []int32{2}) } diff --git a/prep_encoder.go b/prep_encoder.go index 5448c814f..827542c50 100644 --- a/prep_encoder.go +++ b/prep_encoder.go @@ -132,7 +132,6 @@ func (pe *prepEncoder) putStringArray(in []string) error { } func (pe *prepEncoder) putCompactInt32Array(in []int32) error { - if in == nil { return errors.New("expected int32 array to be non null") } @@ -143,7 +142,6 @@ func (pe *prepEncoder) putCompactInt32Array(in []int32) error { } func (pe *prepEncoder) putNullableCompactInt32Array(in []int32) error { - if in == nil { pe.putUVarint(0) return nil diff --git a/real_decoder.go b/real_decoder.go index 0178ba569..8ac576db2 100644 --- a/real_decoder.go +++ b/real_decoder.go @@ -78,7 +78,7 @@ func (rd *realDecoder) getVarint() (int64, error) { func (rd *realDecoder) getUVarint() (uint64, error) { tmp, n := binary.Uvarint(rd.raw[rd.off:]) if n == 0 { - rd.off = int(len(rd.raw)) + rd.off = len(rd.raw) return 0, ErrInsufficientData } @@ -243,7 +243,6 @@ func (rd *realDecoder) getCompactNullableString() (*string, error) { } func (rd *realDecoder) getCompactInt32Array() ([]int32, error) { - n, err := rd.getUVarint() if err != nil { return nil, err From ee24607bb1b934adb00ec3ce94d59fdaaf3e83a3 Mon Sep 17 00:00:00 2001 From: Dirk Wilden Date: Tue, 25 Feb 2020 14:30:15 +0100 Subject: [PATCH 18/20] make sure only controller answers requests --- admin.go | 2 +- admin_test.go | 44 ++++++++++++++++++++++++++++++++++++-------- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/admin.go b/admin.go index 32ba2a02b..8556612af 100644 --- a/admin.go +++ b/admin.go @@ -512,7 +512,7 @@ func (ca *clusterAdmin) ListPartitionReassignments(topic string, partitions []in request.AddBlock(topic, partitions) - b, err := ca.findAnyBroker() + b, err := ca.Controller() if err != nil { return nil, err } diff --git a/admin_test.go b/admin_test.go index bae4de5f7..d8ed18ba9 100644 --- a/admin_test.go +++ b/admin_test.go @@ -336,10 +336,17 @@ func TestClusterAdminAlterPartitionReassignments(t *testing.T) { seedBroker := NewMockBroker(t, 1) defer seedBroker.Close() + secondBroker := NewMockBroker(t, 2) + defer secondBroker.Close() + seedBroker.SetHandlerByMap(map[string]MockResponse{ "MetadataRequest": NewMockMetadataResponse(t). - SetController(seedBroker.BrokerID()). - SetBroker(seedBroker.Addr(), seedBroker.BrokerID()), + SetController(secondBroker.BrokerID()). + SetBroker(seedBroker.Addr(), seedBroker.BrokerID()). + SetBroker(secondBroker.Addr(), secondBroker.BrokerID()), + }) + + secondBroker.SetHandlerByMap(map[string]MockResponse{ "AlterPartitionReassignmentsRequest": NewMockAlterPartitionReassignmentsResponse(t), }) @@ -367,10 +374,17 @@ func TestClusterAdminAlterPartitionReassignmentsWithDiffVersion(t *testing.T) { seedBroker := NewMockBroker(t, 1) defer seedBroker.Close() + secondBroker := NewMockBroker(t, 2) + defer secondBroker.Close() + seedBroker.SetHandlerByMap(map[string]MockResponse{ "MetadataRequest": NewMockMetadataResponse(t). - SetController(seedBroker.BrokerID()). - SetBroker(seedBroker.Addr(), seedBroker.BrokerID()), + SetController(secondBroker.BrokerID()). + SetBroker(seedBroker.Addr(), seedBroker.BrokerID()). + SetBroker(secondBroker.Addr(), secondBroker.BrokerID()), + }) + + secondBroker.SetHandlerByMap(map[string]MockResponse{ "AlterPartitionReassignmentsRequest": NewMockAlterPartitionReassignmentsResponse(t), }) @@ -399,10 +413,17 @@ func TestClusterAdminListPartitionReassignments(t *testing.T) { seedBroker := NewMockBroker(t, 1) defer seedBroker.Close() + secondBroker := NewMockBroker(t, 2) + defer secondBroker.Close() + seedBroker.SetHandlerByMap(map[string]MockResponse{ "MetadataRequest": NewMockMetadataResponse(t). - SetController(seedBroker.BrokerID()). - SetBroker(seedBroker.Addr(), seedBroker.BrokerID()), + SetController(secondBroker.BrokerID()). + SetBroker(seedBroker.Addr(), seedBroker.BrokerID()). + SetBroker(secondBroker.Addr(), secondBroker.BrokerID()), + }) + + secondBroker.SetHandlerByMap(map[string]MockResponse{ "ListPartitionReassignmentsRequest": NewMockListPartitionReassignmentsResponse(t), }) @@ -437,10 +458,17 @@ func TestClusterAdminListPartitionReassignmentsWithDiffVersion(t *testing.T) { seedBroker := NewMockBroker(t, 1) defer seedBroker.Close() + secondBroker := NewMockBroker(t, 2) + defer secondBroker.Close() + seedBroker.SetHandlerByMap(map[string]MockResponse{ "MetadataRequest": NewMockMetadataResponse(t). - SetController(seedBroker.BrokerID()). - SetBroker(seedBroker.Addr(), seedBroker.BrokerID()), + SetController(secondBroker.BrokerID()). + SetBroker(seedBroker.Addr(), seedBroker.BrokerID()). + SetBroker(secondBroker.Addr(), secondBroker.BrokerID()), + }) + + secondBroker.SetHandlerByMap(map[string]MockResponse{ "ListPartitionReassignmentsRequest": NewMockListPartitionReassignmentsResponse(t), }) From 6f230b4692194f24f1d9a44f9c2061489972c5d0 Mon Sep 17 00:00:00 2001 From: Dirk Wilden Date: Tue, 25 Feb 2020 14:33:17 +0100 Subject: [PATCH 19/20] use correct default for timeout --- admin.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/admin.go b/admin.go index 8556612af..bcb461015 100644 --- a/admin.go +++ b/admin.go @@ -457,7 +457,7 @@ func (ca *clusterAdmin) AlterPartitionReassignments(topic string, assignment [][ } request := &AlterPartitionReassignmentsRequest{ - TimeoutMs: int32(10000), + TimeoutMs: int32(60000), Version: int16(0), } @@ -506,7 +506,7 @@ func (ca *clusterAdmin) ListPartitionReassignments(topic string, partitions []in } request := &ListPartitionReassignmentsRequest{ - TimeoutMs: int32(10000), + TimeoutMs: int32(60000), Version: int16(0), } From 9e0199b6b39ab115f0b65be3a2355835cd5aa2d1 Mon Sep 17 00:00:00 2001 From: Dirk Wilden Date: Wed, 4 Mar 2020 14:52:52 +0100 Subject: [PATCH 20/20] modify test to skip byteComparison --- list_partition_reassignments_request_test.go | 16 +--------------- request_test.go | 10 +++++++++- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/list_partition_reassignments_request_test.go b/list_partition_reassignments_request_test.go index d50b35a26..d9f9f92ca 100644 --- a/list_partition_reassignments_request_test.go +++ b/list_partition_reassignments_request_test.go @@ -11,20 +11,6 @@ var ( 0, 0, 0, 0, // partitionId 0, 0, // empty tagged fields } - - listPartitionReassignmentsRequestTwoBlocks = []byte{ - 0, 0, 39, 16, // timout 10000 - 3, // 3-1=2 blocks - 6, 116, 111, 112, 105, 99, // topic name "topic" as compact string - 2, // 2-1=1 partitions - 0, 0, 0, 0, // partitionId - 0, // empty tagged fields - 7, 116, 111, 112, 105, 99, 50, // topic name "topic2" as compact string - 3, // 3-1=2 partitions - 0, 0, 0, 1, // partitionId - 0, 0, 0, 2, // partitionId - 0, 0, // empty tagged fields - } ) func TestListPartitionReassignmentRequest(t *testing.T) { @@ -41,5 +27,5 @@ func TestListPartitionReassignmentRequest(t *testing.T) { request.AddBlock("topic2", []int32{1, 2}) - testRequest(t, "two blocks", request, listPartitionReassignmentsRequestTwoBlocks) + testRequestWithoutByteComparison(t, "two blocks", request) } diff --git a/request_test.go b/request_test.go index fdfe79eab..95cd6bb32 100644 --- a/request_test.go +++ b/request_test.go @@ -42,6 +42,14 @@ func testRequest(t *testing.T, name string, rb protocolBody, expected []byte) { testRequestDecode(t, name, rb, packet) } +func testRequestWithoutByteComparison(t *testing.T, name string, rb protocolBody) { + if !rb.requiredVersion().IsAtLeast(MinVersion) { + t.Errorf("Request %s has invalid required version", name) + } + packet := testRequestEncode(t, name, rb, nil) + testRequestDecode(t, name, rb, packet) +} + func testRequestEncode(t *testing.T, name string, rb protocolBody, expected []byte) []byte { req := &request{correlationID: 123, clientID: "foo", body: rb} packet, err := encode(req, nil) @@ -59,7 +67,7 @@ func testRequestEncode(t *testing.T, name string, rb protocolBody, expected []by if err != nil { t.Error(err) - } else if !bytes.Equal(packet[headerSize:], expected) { + } else if expected != nil && !bytes.Equal(packet[headerSize:], expected) { t.Error("Encoding", name, "failed\ngot ", packet[headerSize:], "\nwant", expected) } return packet