Skip to content

Commit

Permalink
Make rd_kafka_destroy a safe foreign call
Browse files Browse the repository at this point in the history
- It blocks until everything is being destroyed
- It might trigger callbacks

Documentation from librdkafka:

```
/**
 * @brief Destroy Kafka handle.
 *
 * @remark This is a blocking operation.
 * @remark rd_kafka_consumer_close() will be called from this function
 *         if the instance type is RD_KAFKA_CONSUMER, a \c group.id was
 *         configured, and the rd_kafka_consumer_close() was not
 *         explicitly called by the application. This in turn may
 *         trigger consumer callbacks, such as rebalance_cb.
 *         Use rd_kafka_destroy_flags() with
 *         RD_KAFKA_DESTROY_F_NO_CONSUMER_CLOSE to avoid this behaviour.
 *
 * @sa rd_kafka_destroy_flags()
 */
RD_EXPORT
void        rd_kafka_destroy(rd_kafka_t *rk);
```
  • Loading branch information
alexbiehl authored Aug 9, 2019
1 parent 40fef1e commit 94a2898
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/Kafka/Internal/RdKafka.chs
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ newRdKafkaTopicConfT = do
{enumToCInt `RdKafkaTypeT', `RdKafkaConfTPtr', id `CCharBufPointer', cIntConv `CSize'}
-> `RdKafkaTPtr' #}

foreign import ccall unsafe "rdkafka.h &rd_kafka_destroy"
foreign import ccall safe "rdkafka.h &rd_kafka_destroy"
rdKafkaDestroy :: FunPtr (Ptr RdKafkaT -> IO ())

newRdKafkaT :: RdKafkaTypeT -> RdKafkaConfTPtr -> IO (Either Text RdKafkaTPtr)
Expand Down

3 comments on commit 94a2898

@AlexeyRaga
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexbiehl I have started experiencing a strange problem: some of my jobs would hang on closing kafka consumers. Not often, but sometimes.
Do you think that it could be related to this change?
It is a bit puzzling to be because we don't pass anything heap-allocated into that function, but it also that weird destroy function that is being attached, could it be that GH plays some role in this hangs?

@alexbiehl
Copy link
Contributor Author

@alexbiehl alexbiehl commented on 94a2898 Oct 21, 2019 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexeyRaga
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexbiehl @shlevy to be honest, I don't really know if the patch is responsible, I just don't remember seeing this problem before. I will need to dig into stats to see if it was indeed happening earlier...

Please sign in to comment.