Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/3278
Tests have passed, failed ones are Travis timeouts. Merging this now ...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your proje
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/3278
Addressed waiting for flushing to start with a latch instead of sleeping.
Will merge this to `master` and `release-1.2` once Travis turns green
(running also locally due to the timeouts).
---
I
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/3278
@tillrohrmann thanks for your second-pass review, the tips you mentioned
were helpful. I've incoporated all of your comments except:
> Maybe you could override the DummyFlinkKafkaProducer#fl
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/3278
@tillrohrmann I've hopefully addressed your comments. Could you please have
a look?
The major change was to refactor the `DummyFlinkKafkaProducer` so that a
simple mock `KafkaProducer` can
Github user rmetzger commented on the issue:
https://github.com/apache/flink/pull/3278
I think we should allow users to disable the wait on flush, because it can
substantially delay the confirmation of a checkpoint.
If a user favors fast checkpoints over complete data in Kafka (for
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/3278
Thanks for the detailed review @tillrohrmann, I'll follow-up and address
your comments.
Regarding removing the `setFlushOnCheckpoint`:
I think it was added at first to provide flexibilit
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/3278
Instead of directly re-adding
`testAtLeastOnceProducerFailsIfFlushingDisabled `, I instead added a test
`testDoesNotWaitForPendingRecordsIfFlushingDisabled` to simply assure that the
snapshot metho
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/3278
@tillrohrmann thanks for the comment. I'll try again and see if I can come
with up a proper test for `testAtLeastOnceProdcuerFailsIfFlushingDisabled()`.
---
If your project is set up for it, you ca
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/3278
Ah now I understand what the problem of
`testAtLeastOnceProdcuerFailsIfFlushingDisabled` was. Can't we mock the
`KafkaProducer` to control when the record's callbacks are triggered? That way
we
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/3278
@tillrohrmann
Yes, in the condition that you described, then at-least-once doesn't hold.
I said _might_ mainly considering there is chance that for every checkpoint
barrier, the previous events
Github user tillrohrmann commented on the issue:
https://github.com/apache/flink/pull/3278
I don't think that you have at least once guarantees if you disable
flushing. Assume the following: You have the input `event1, checkpoint barrier,
event2`. Now you write `event1` to Kafka but i
11 matches
Mail list logo
