vinaynb commented 6 years ago

Description

Kafka producer should throw exception on the client when we have min.insync.replicas=2 the partition leader node crashes unexpectedly in a 2 node kafka cluster but instead it keeps sending messages to cluster and consumer is able to pull them successfully as well.

How to reproduce

My setup

1 zookeper node 2 kafka broker nodes (identical config except id,log path and listeners property) 1 producer (doing async writes) and 1 subscriber written in go using this library I am creating a topic using kafka's command line tool as below

./kafka-topics.sh --zookeeper localhost:2181 --create --topic foo --partitions 1 --replication-factor 2 --config min.insync.replicas=2

The issue is that whenever i kill leader node of the partition, the producer and consumer still keep on pushing and pulling messages from the kafka cluster even though the min.insync.replicas setting for my topic is 2. I expect producer to throw exceptions and partition should not be allowed for writing as per the docs.

I found one more thread similar to mine wherein it was suggested to set min.insync.replicas per topic which i have done but still there are no errors on producer

Am i doing something wrong somewhere ?

Producer code

func main() {
    kafkaProducer, kafkaConErr = kafka.NewProducer( & kafka.ConfigMap {
        "bootstrap.servers": "localhost:9092",
        "acks": "-1"
    })
    if kafkaConErr != nil {
        fmt.Println("Error creating InfluxDB Client: ", kafkaConErr.Error())
    }
    defer kafkaProducer.Close()

    topic: = "foo"
        /* for range []string{"Welcome", "to", "the", "Confluent", "Kafka", "Golang", "client"} { */
    perr: = kafkaProducer.Produce( & kafka.Message {
        TopicPartition: kafka.TopicPartition {
            Topic: & topic,
            Partition: kafka.PartitionAny
        },
        Value: empData,
    }, nil)
    if perr != nil {
        fmt.Println(err.Error())
        return
    }
    deliveryReportHandler()

}

func deliveryReportHandler() {
    // Delivery report handler for produced messages
    go func() {
        for e: = range kafkaProducer.Events() {
            switch ev: = e.(type) {
                case *kafka.Message:
                    if ev.TopicPartition.Error != nil {
                        fmt.Printf("Delivery failed: %v\n", ev.TopicPartition)
                    } else {
                        fmt.Printf("Delivered message to topic %s [%d] at offset %v\n", * ev.TopicPartition.Topic, ev.TopicPartition.Partition, ev.TopicPartition.Offset)
                    }
                default:
                    fmt.Printf("Ignored event: %s\n", ev)
            }
        }
    }()
}

My broker config as as below

############################# Server Basics #############################

# The id of the broker. This must be set to a unique integer for each broker.
broker.id=0

############################# Socket Server Settings #############################

# The address the socket server listens on. It will get the value returned from 
# java.net.InetAddress.getCanonicalHostName() if not configured.
#   FORMAT:
#     listeners = listener_name://host_name:port
#   EXAMPLE:
#     listeners = PLAINTEXT://your.host.name:9092
listeners=PLAINTEXT://:9092

# Hostname and port the broker will advertise to producers and consumers. If not set, 
# it uses the value for "listeners" if configured.  Otherwise, it will use the value
# returned from java.net.InetAddress.getCanonicalHostName().
#advertised.listeners=PLAINTEXT://your.host.name:9092

# Maps listener names to security protocols, the default is for them to be the same. See the config documentation for more details
#listener.security.protocol.map=PLAINTEXT:PLAINTEXT,SSL:SSL,SASL_PLAINTEXT:SASL_PLAINTEXT,SASL_SSL:SASL_SSL

# The number of threads that the server uses for receiving requests from the network and sending responses to the network
num.network.threads=3

# The number of threads that the server uses for processing requests, which may include disk I/O
num.io.threads=8

# The send buffer (SO_SNDBUF) used by the socket server
socket.send.buffer.bytes=102400

# The receive buffer (SO_RCVBUF) used by the socket server
socket.receive.buffer.bytes=102400

# The maximum size of a request that the socket server will accept (protection against OOM)
socket.request.max.bytes=104857600

############################# Log Basics #############################

# A comma separated list of directories under which to store log files
log.dirs=/tmp/kafka-logs

# The default number of log partitions per topic. More partitions allow greater
# parallelism for consumption, but this will also result in more files across
# the brokers.
num.partitions=1

# The number of threads per data directory to be used for log recovery at startup and flushing at shutdown.
# This value is recommended to be increased for installations with data dirs located in RAID array.
num.recovery.threads.per.data.dir=1

############################# Internal Topic Settings  #############################
# The replication factor for the group metadata internal topics "__consumer_offsets" and "__transaction_state"
# For anything other than development testing, a value greater than 1 is recommended for to ensure availability such as 3.
#offsets.topic.replication.factor=1
transaction.state.log.replication.factor=1
transaction.state.log.min.isr=1

############################# Log Flush Policy #############################

# Messages are immediately written to the filesystem but by default we only fsync() to sync
# the OS cache lazily. The following configurations control the flush of data to disk.
# There are a few important trade-offs here:
#    1. Durability: Unflushed data may be lost if you are not using replication.
#    2. Latency: Very large flush intervals may lead to latency spikes when the flush does occur as there will be a lot of data to flush.
#    3. Throughput: The flush is generally the most expensive operation, and a small flush interval may lead to excessive seeks.
# The settings below allow one to configure the flush policy to flush data after a period of time or
# every N messages (or both). This can be done globally and overridden on a per-topic basis.

# The number of messages to accept before forcing a flush of data to disk
#log.flush.interval.messages=10000

# The maximum amount of time a message can sit in a log before we force a flush
#log.flush.interval.ms=1000

############################# Log Retention Policy #############################

# The following configurations control the disposal of log segments. The policy can
# be set to delete segments after a period of time, or after a given size has accumulated.
# A segment will be deleted whenever *either* of these criteria are met. Deletion always happens
# from the end of the log.

# The minimum age of a log file to be eligible for deletion due to age
log.retention.hours=168

# A size-based retention policy for logs. Segments are pruned from the log unless the remaining
# segments drop below log.retention.bytes. Functions independently of log.retention.hours.
#log.retention.bytes=1073741824

# The maximum size of a log segment file. When this size is reached a new log segment will be created.
log.segment.bytes=1073741824

# The interval at which log segments are checked to see if they can be deleted according
# to the retention policies
log.retention.check.interval.ms=300000

############################# Zookeeper #############################

# Zookeeper connection string (see zookeeper docs for details).
# This is a comma separated host:port pairs, each corresponding to a zk
# server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002".
# You can also append an optional chroot string to the urls to specify the
# root directory for all kafka znodes.
zookeeper.connect=localhost:2181

# Timeout in ms for connecting to zookeeper
zookeeper.connection.timeout.ms=6000

############################# Group Coordinator Settings #############################

# The following configuration specifies the time, in milliseconds, that the GroupCoordinator will delay the initial consumer rebalance.
# The rebalance will be further delayed by the value of group.initial.rebalance.delay.ms as new members join the group, up to a maximum of max.poll.interval.ms.
# The default value for this is 3 seconds.
# We override this to 0 here as it makes for a better out-of-the-box experience for development and testing.
# However, in production environments the default value of 3 seconds is more suitable as this will help to avoid unnecessary, and potentially expensive, rebalances during application startup.
group.initial.rebalance.delay.ms=0

auto.leader.rebalance.enable=true

leader.imbalance.check.interval.seconds=5

leader.imbalance.per.broker.percentage=0

min.insync.replicas=2

offsets.topic.replication.factor=2

replica.lag.time.max.ms=1000

Note - this question was originally asked on stackoverflow (link)

Checklist

Please provide the following information:

[ ] confluent-kafka-go and librdkafka version (LibraryVersion()): 0.11.4 & 0.11.5
[ ] Apache Kafka broker version: 2.0.0 (commit - 3402a8361b734732)
[ ] Client configuration: ConfigMap{...}
[ ] Operating system: ubuntu 14.04
[ ] Provide client logs (with "debug": ".." as necessary)
[ ] Provide broker log excerpts
[ ] Critical issue

edenhill commented 6 years ago

Are you getting any delivery reports to your handler? And if so; is it with error set or not?

vinaynb commented 6 years ago

yes i am getting delivery reports and they say messages are delivered successfully with offset numbers.

What do you mean when you say "with error set or not" ? How can i check if error is set or not ?

fwiw i am getting those messages on the consumer end as well so i assume that they are being delivered successfully.

edenhill commented 6 years ago

Could you try setting "acks" on the default.topic.config map instead, like so:

kafka.NewProducer( & kafka.ConfigMap {
        "bootstrap.servers": "localhost:9092",
        "default.topic.config": kafka.ConfigMap{"acks": "all"}
    })

rnpridgeon commented 6 years ago

For completeness I'm including the following doc snippet for min.insync.replicas

When a producer sets acks to "all" (or "-1"), min.insync.replicas specifies the minimum number of replicas that must acknowledge a write for the write to be considered successful.

vinaynb commented 6 years ago

@edenhill yes in can confirm that it works if i set acks default.topic.config on as you said. Producer now fails as expected with message Delivery failed: foo[0]@unset(Broker: Not enough in-sync replicas)

So i guess this a bug ?

rnpridgeon commented 6 years ago

@vinaynb although a tad bit confusing this is actually the expected behavior. Please find the following snippet from librdkafka's configuration.md

This field indicates how many acknowledgements the leader broker must receive from ISR brokers before responding to the request: 0=Broker does not send any response/ack to client, 1=Only the leader broker will need to ack the message, -1 or all=broker will block until message is committed by all in sync replicas (ISRs) or broker's min.insync.replicas setting before sending response.

https://github.com/edenhill/librdkafka/blob/master/CONFIGURATION.md

vinaynb commented 6 years ago

@rnpridgeon sorry but i didn't understand your comment. As per my question i have min.insync.replicas = 2 in my topic and acks=-1 in my producer and if leader node fails, shouldn't it throw exception straightaway ?

rnpridgeon commented 6 years ago

@vinaynb I apologize I thought you were previously not setting acks, going back to the top I see you actually have it set to -1. The fact the go client is silently ignoring this is actually a bug. Actually golag shouldn't rely on a separate topic configuration at all now that librdkafka handles them both in the same configuration object.

Sorry I skimmed the initial request too quickly.

thangld322 commented 5 years ago

Have this bug been fixed? How can I set acks = 1 to my kafka producer and it actually works? Thanks.

edenhill commented 5 years ago

Fix is to apply all default.topic.config properties on the parent map and don't meddle with the rdkafka default topic config at all.

tobgu commented 5 years ago

@edenhill I had a problem like the one described here where messages were lost during chaos-like (eg. removing cluster nodes during load) testing due to the producer not respecting acks.

The failing producer config was:

&kafka.ConfigMap{
                ...
        "acks":              "all",
                ...
}

Only by updating it to be like below the issue was resolved:

&kafka.ConfigMap{
                ...
        "acks":              "all",
        "default.topic.config":  kafka.ConfigMap{"acks": "all"},
                ...
}

This is on version v0.11.6. With v1.0+ default.topic.config has been deprecated. Have any changes/updates been done in v1.0 that should make the above work without explicitly setting the acks property on the default.topic.config? I want to upgrade to a more recent version but am a bit worried that the problem will be re-introduced if I remove the current ack config on topic level.

edenhill commented 5 years ago

Yes this is fixed in v1.0.0, you should now put all the topic configs on the main ConfigMap and not specify a default.topic.config (since it will overwrite the topic-configs on main)

edenhill commented 5 years ago

You can verify this by setting "message.timeout.ms" to 1234 and try to produce to an unavailable broker (i.e., set bootstrap.servers to "blabla"), your messages should time out within a second or two instead of the default 5 minutes.

tobgu commented 5 years ago

OK, thanks for the quick response!

confluentinc / confluent-kafka-go

Kafka producer unexpected behaviour #235

Description

How to reproduce

Checklist