Kafka 11 - record headers support

habutre commented 4 years ago

This PR is targeted to #397 to provide support for record headers provided since Kafka 0.11

The headers were implemented first on Kayrock and for testing purposes this branch points to Kayrock master. After a new Kayrock be released this PR can be changed and merged. This change is only for your review and validation.

To maintain compability while using the new client the Protocol.{Produce, Fetch}.Message had to be changed.

Only the new client is aware about such info on messages and may not cause conflicts or misbehavior

Out of scope: The mix format causes some changes that should be evaluate if make sense or have to be reverted

habutre commented 4 years ago

Hey @dantswain @jbruggem @joshuawscott @bjhaid I think you are the right guys to involve here to help me (let me know if someone might be added or removed from my mentions)

I am facing random errors with ci_tests.sh also with all_tests.sh locally, did you have some tips & tricks that I can apply to get rid of it and making the tests pass?

One can see in the CI tests each config has erros in different test cases and I am a bit lost to find a solution.

Thanks for any help.

joshuawscott commented 4 years ago

When I released 0.11.0 I fixed most of the intermittent failures, I thought. I was able to run 20x in a row without any failures locally. That said, because Travis uses 1.5 core machines, it doesn't necessarily work the same. There does seem to be a completely consistent failure, though:

  1) test gzip compression - produce v3, fetch v0 (KafkaEx.KayrockRecordBatchTest)
     test/integration/kayrock/record_batch_test.exs:301
     ** (UndefinedFunctionError) function nil.value/0 is undefined
     code: assert message.value == msg
     stacktrace:
       nil.value()
       test/integration/kayrock/record_batch_test.exs:328: (test)
     The following output was logged:

     21:27:44.845 [debug] Successfully connected to broker "localhost":9093

     21:27:44.888 [debug] Successfully connected to broker "localhost":9094

     21:27:44.932 [debug] Successfully connected to broker "localhost":9095

Often a single failure will cause ripples due to using the same underlying brokers, so my suggestion would be to solve this failure and see if the others begin to work.

habutre commented 4 years ago

Hey @dantswain I would really appreciate if you could take a look into my last commit that make the ci tests pass. I am a bit affraid to change pre-existent tests to make all them pass.

@joshuawscott I think now you can review and if you can help to clarify about the offsets manipulation on kafka_ex/test/integration/kayrock/record_batch_test.exs will be awesome

:information_source: Just a remind I am pointing the Kayrock to master and a release is necessary there before move forward with this PR

joshuawscott commented 3 years ago

kayrock 0.1.12 is out now

dantswain commented 3 years ago

@habutre I apologize for not getting to this yet. I have been unbelievably busy :( I will try to make time.

habutre commented 3 years ago

Is it possible to retry the CI? Checking the errors 1) timeout 2) failed to get deps

Argonus commented 3 years ago

@habutre @joshuawscott Are we blocked by sth here? Maybe it's worth merging master to this PR & if everything is green merge it to the main branch. If I can help somehow, please let me know.

habutre commented 3 years ago

@habutre @joshuawscott Are we blocked by sth here? Maybe it's worth merging master to this PR & if everything is green merge it to the main branch. If I can help somehow, please let me know.

Hey @Argonus this PR is ready for review and I am keeping as much as possible synced with latest merged PRs.

Argonus commented 2 years ago

@jbruggem @joshuawscott @dantswain Can I help somehow with this PR?

jbruggem commented 2 years ago

@habutre do you wish to commit those last documentation suggestions ?

Once done, I'll merge.

Argonus commented 2 years ago

@jbruggem Maybe it's worth releasing a new version as well? There would be two quite big things: support for snappy & support for headers

jbruggem commented 2 years ago

@Argonus TBH I've never cut a release on KafkaEx before, so I'm not sure what the criteria are :).

@dantswain @joshuawscott would this be a good moment to make a release, and if so who can do it ?

kafkaex / kafka_ex

Kafka 11 - record headers support #414