Skip to content

Commit

Permalink
Fix the kafka panic when sending the message to a closed channel (mil…
Browse files Browse the repository at this point in the history
…vus-io#25116) (milvus-io#25117)

Signed-off-by: SimFG <bang.fu@zilliz.com>
  • Loading branch information
SimFG authored and zhuwenxing committed Jul 25, 2023
1 parent 65c4800 commit 4d3dc68
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 3 deletions.
17 changes: 14 additions & 3 deletions pkg/mq/msgstream/mqwrapper/kafka/kafka_producer.go
Expand Up @@ -7,20 +7,20 @@ import (
"time"

"github.com/confluentinc/confluent-kafka-go/kafka"
"go.uber.org/zap"

"github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/log"
"github.com/milvus-io/milvus/pkg/metrics"
"github.com/milvus-io/milvus/pkg/mq/msgstream/mqwrapper"
"github.com/milvus-io/milvus/pkg/util/timerecord"
"go.uber.org/zap"
)

type kafkaProducer struct {
p *kafka.Producer
topic string
deliveryChan chan kafka.Event
closeOnce sync.Once
isClosed bool
}

func (kp *kafkaProducer) Topic() string {
Expand All @@ -31,6 +31,12 @@ func (kp *kafkaProducer) Send(ctx context.Context, message *mqwrapper.ProducerMe
start := timerecord.NewTimeRecorder("send msg to stream")
metrics.MsgStreamOpCounter.WithLabelValues(metrics.SendMsgLabel, metrics.TotalLabel).Inc()

if kp.isClosed {
metrics.MsgStreamOpCounter.WithLabelValues(metrics.SendMsgLabel, metrics.FailLabel).Inc()
log.Error("kafka produce message fail because the producer has been closed", zap.String("topic", kp.topic))
return nil, common.NewIgnorableError(fmt.Errorf("kafka producer is closed"))
}

headers := make([]kafka.Header, 0, len(message.Properties))
for key, value := range message.Properties {
header := kafka.Header{Key: key, Value: []byte(value)}
Expand Down Expand Up @@ -69,9 +75,14 @@ func (kp *kafkaProducer) Send(ctx context.Context, message *mqwrapper.ProducerMe

func (kp *kafkaProducer) Close() {
kp.closeOnce.Do(func() {
kp.isClosed = true

start := time.Now()
//flush in-flight msg within queue.
kp.p.Flush(10000)
i := kp.p.Flush(10000)
if i > 0 {
log.Warn("There are still un-flushed outstanding events", zap.Int("event_num", i), zap.Any("topic", kp.topic))
}

close(kp.deliveryChan)

Expand Down
27 changes: 27 additions & 0 deletions pkg/mq/msgstream/mqwrapper/kafka/kafka_producer_test.go
Expand Up @@ -67,3 +67,30 @@ func TestKafkaProducer_SendFail(t *testing.T) {
producer.Close()
}
}

func TestKafkaProducer_SendFailAfterClose(t *testing.T) {
kafkaAddress := getKafkaBrokerList()
kc := NewKafkaClientInstance(kafkaAddress)
defer kc.Close()
assert.NotNil(t, kc)

rand.Seed(time.Now().UnixNano())
topic := fmt.Sprintf("test-topic-%d", rand.Int())

producer, err := kc.CreateProducer(mqwrapper.ProducerOptions{Topic: topic})
assert.Nil(t, err)
assert.NotNil(t, producer)

producer.Close()

kafkaProd := producer.(*kafkaProducer)
assert.Equal(t, kafkaProd.Topic(), topic)

msg2 := &mqwrapper.ProducerMessage{
Payload: []byte{},
Properties: map[string]string{},
}
_, err = producer.Send(context.TODO(), msg2)
time.Sleep(10 * time.Second)
assert.NotNil(t, err)
}

0 comments on commit 4d3dc68

Please sign in to comment.