MF-703 - Reliably Publish Event Messages to Redis #1836

rodneyosodo · 2023-06-28T17:52:19Z

What does this do?

I have introduced a mechanism for storing unpublished events and implementing retries. We use memory to save unpublished events when the Redis server is unavailable. Then, we periodically attempt to publish the stored events when the Redis server becomes available again.

When an event needs to be published, check if the Redis server is reachable. Added a 100ms timeout so stop retrying to connect to Redis when it is down
If the Redis server is reachable, publish the event.
If the Redis server is unreachable, store the event in memory using slice.
Periodically check the connection to the Redis server, after every minute, and retry publishing the stored events when the connection is re-established.
Once an event is successfully published, remove it from memory.

Which issue(s) does this PR fix/relate to?

Resolves #703

List any changes that modify/break current functionality

added 100ms timeout to check if Redis is reachable

Have you included tests for your changes?

No

Did you document any new/modified functionality?

No

Notes

Tested by pausing the es-redis container and resuming it after doing some operations using the e2e tool.

codecov · 2023-06-28T18:06:16Z

Codecov Report

Merging #1836 (d5c598c) into master (3e7bac4) will increase coverage by 0.56%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master    #1836      +/-   ##
==========================================
+ Coverage   67.12%   67.69%   +0.56%     
==========================================
  Files         118      118              
  Lines        9438     9434       -4     
==========================================
+ Hits         6335     6386      +51     
+ Misses       2436     2374      -62     
- Partials      667      674       +7

Files Changed	Coverage Δ
bootstrap/redis/producer/streams.go	`80.53% <75.00%> (+24.12%)`	⬆️
bootstrap/redis/producer/events.go	`78.68% <100.00%> (+24.59%)`	⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

SammyOina · 2023-07-11T12:33:19Z

internal/clients/redis/producer.go

+
+type eventStore struct {
+	client            *redis.Client
+	unpublishedEvents []*redis.XAddArgs


this can grow unexpectedly large sizes. I think a different approach is required for memory management. Perhaps a channel or some external queue like nats might serve better

Currently, nats is configured as fire and forget unless we move to Jetstream. I have implemented a channel with a max of 1M capacity.

Before publishing the memory footprint of the container was 17MB after publishing before resuming Redis it was 49MB

dborovcanin

I'm afraid that reliable publishing needs a little bit more than what we have. The buffer mechanism is not bad in itself, but what it effectively introduce is - the reordering of events. This topic is very advanced and out of the scope of this PR. What we can do is - provide a mechanism to mark these events "dirty" or "out of order" maybe by adding a field such as event_time that will represent the time event occurred and will be different from the time the event is published on Redis.

dborovcanin

Please fix tests and test locally.

drasko · 2023-07-28T16:18:41Z

@dborovcanin please review this one

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

SammyOina

LGTM

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

drasko

LGTM

* Reliably Publish Event Messages to Redis Signed-off-by: rodneyosodo <blackd0t@protonmail.com> * Make Redis Producer Safe For Concurrent Use Signed-off-by: rodneyosodo <blackd0t@protonmail.com> * Combine Redis Publishing Signed-off-by: rodneyosodo <blackd0t@protonmail.com> * Add defer statement Signed-off-by: rodneyosodo <blackd0t@protonmail.com> * Use Channel Instead of Array Signed-off-by: rodneyosodo <blackd0t@protonmail.com> * Adding `occurred_at` Signed-off-by: rodneyosodo <blackd0t@protonmail.com> * Fix Check `occurred_at` Signed-off-by: rodneyosodo <blackd0t@protonmail.com> * Remove Unused Const Signed-off-by: rodneyosodo <blackd0t@protonmail.com> * Check For Non NIL Error on Publishing Signed-off-by: rodneyosodo <blackd0t@protonmail.com> * Add More Tests Signed-off-by: rodneyosodo <blackd0t@protonmail.com> * Hanndle When Channel Is Full Signed-off-by: rodneyosodo <blackd0t@protonmail.com> * Fix Issue After Rebase Signed-off-by: rodneyosodo <blackd0t@protonmail.com> * Fix Tests Signed-off-by: rodneyosodo <blackd0t@protonmail.com> --------- Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

rodneyosodo requested a review from a team as a code owner June 28, 2023 17:52

rodneyosodo force-pushed the MF-703 branch 4 times, most recently from e3cc95a to 6f0b3c2 Compare July 10, 2023 12:25

SammyOina requested changes Jul 11, 2023

View reviewed changes

dborovcanin requested changes Jul 11, 2023

View reviewed changes

dborovcanin force-pushed the MF-703 branch from 2a114cd to 57d5dc2 Compare July 11, 2023 14:52

rodneyosodo force-pushed the MF-703 branch 2 times, most recently from 7edae13 to 32d0120 Compare July 11, 2023 15:01

dborovcanin requested changes Jul 11, 2023

View reviewed changes

rodneyosodo force-pushed the MF-703 branch 3 times, most recently from 93525f1 to b84c8d2 Compare July 12, 2023 10:15

SammyOina approved these changes Jul 12, 2023

View reviewed changes

rodneyosodo force-pushed the MF-703 branch 5 times, most recently from 5e400d3 to b230fe8 Compare July 13, 2023 15:07

dborovcanin force-pushed the MF-703 branch from ee2d468 to 07d93aa Compare July 19, 2023 10:18

rodneyosodo force-pushed the MF-703 branch 7 times, most recently from 69320c3 to 970da5b Compare July 28, 2023 13:51

rodneyosodo added 6 commits July 31, 2023 17:24

Reliably Publish Event Messages to Redis

e4e91fd

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

Make Redis Producer Safe For Concurrent Use

d067a0a

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

Combine Redis Publishing

3878993

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

Add defer statement

31d42b2

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

Use Channel Instead of Array

7cc7ff8

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

Adding occurred_at

fb0fc4e

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

SammyOina self-requested a review July 31, 2023 14:24

SammyOina approved these changes Jul 31, 2023

View reviewed changes

rodneyosodo added 6 commits July 31, 2023 17:25

Fix Check occurred_at

fc309e3

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

Remove Unused Const

2b49a6e

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

Check For Non NIL Error on Publishing

fc2399d

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

Add More Tests

1525848

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

Hanndle When Channel Is Full

c0ca2bd

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

Fix Issue After Rebase

587bd7c

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

rodneyosodo force-pushed the MF-703 branch from b0e9f5a to 587bd7c Compare July 31, 2023 14:29

Fix Tests

d5c598c

Signed-off-by: rodneyosodo <blackd0t@protonmail.com>

drasko approved these changes Jul 31, 2023

View reviewed changes

drasko merged commit abc1e3d into absmach:master Jul 31, 2023
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MF-703 - Reliably Publish Event Messages to Redis #1836

MF-703 - Reliably Publish Event Messages to Redis #1836

rodneyosodo commented Jun 28, 2023

codecov bot commented Jun 28, 2023 •

edited

Loading

SammyOina Jul 11, 2023

rodneyosodo Jul 11, 2023

rodneyosodo Jul 11, 2023

dborovcanin left a comment

dborovcanin left a comment

drasko commented Jul 28, 2023

SammyOina left a comment

drasko left a comment

MF-703 - Reliably Publish Event Messages to Redis #1836

MF-703 - Reliably Publish Event Messages to Redis #1836

Conversation

rodneyosodo commented Jun 28, 2023

What does this do?

Which issue(s) does this PR fix/relate to?

List any changes that modify/break current functionality

Have you included tests for your changes?

Did you document any new/modified functionality?

Notes

codecov bot commented Jun 28, 2023 • edited Loading

Codecov Report

SammyOina Jul 11, 2023

Choose a reason for hiding this comment

rodneyosodo Jul 11, 2023

Choose a reason for hiding this comment

rodneyosodo Jul 11, 2023

Choose a reason for hiding this comment

dborovcanin left a comment

Choose a reason for hiding this comment

dborovcanin left a comment

Choose a reason for hiding this comment

drasko commented Jul 28, 2023

SammyOina left a comment

Choose a reason for hiding this comment

drasko left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 28, 2023 •

edited

Loading