Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable fairness on Kinesis Source's SynchronousQueue #105

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

istreeter
Copy link
Contributor

This fixes a possible problem in which some Kinesis shards progress more quickly than others. Each KCL shard processor has its own thread, and each calls queue.put() on the synchronous queue. It is possible that by luck some shard/threads were having their put() accepted by a consumer more quickly.

This fixes a possible problem in which some Kinesis shards progress more
quickly than others. Each KCL shard processor has its own thread, and
each calls `queue.put()` on the synchronous queue.  It is possible that
by luck some shard/threads were having their `put()` accepted by a
consumer more quickly.
@istreeter istreeter force-pushed the kinesis-queue-fairness branch from 8fcbbc6 to 6132477 Compare January 8, 2025 09:38
@istreeter istreeter merged commit aa35e08 into develop Jan 9, 2025
1 check passed
@istreeter istreeter deleted the kinesis-queue-fairness branch January 9, 2025 10:40
istreeter added a commit that referenced this pull request Jan 9, 2025
Currently in common-streams apps, the _application_ (i.e. not this
library) is responsible for tracking latency of events consumed from the
stream.  Because of the way it is implemented in the apps, if an event
gets stuck (e.g. cannot be written to destination) then the latency
drops to zero for the period when the app is retrying the stuck event.

This commit aims to fix the problem where the `latency_millis` metric
wrongly drops to zero:

1. The `SourceAndAck` is now responsible for tracking the latency metric
and pushing the metric to the app. This consolidates the metric across
multiple apps into one place

2. The Metrics class allows metric starting values to be wrapped in and
Effect, i.e. `F[Metrics.State]`. This means the application can pass in
the latency reported by the Source.

3. The `TokenedEvents` now does not contain a stream timestamp.  The
downstream app has no business knowing the stream timestamp, now that
latency is calculated inside common-streams lib.

4. The `LowLevelSource` no longer provides a `lastLiveness`.  Instead,
the low level source is expected to periodically emit a `None` whenever
it is healthy. The higher level `SourceAndAck` then takes responsibility
for monitoring whether the low level source is healthy.

5. As a result of these changes, the `SourceAndAck` emits a latency
metric of zero whenever the source is genuinely healthy but there are no
events to be pulled. This means the app's latency metric is only zero if
there are no events.  This is better than the previous situation, where
the app might mis-leadingly emit a zero latency when the app is stuck,
e.g. cannot connect to warehouse.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants