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

[docs-only] Events adr #3070

Merged
merged 6 commits into from
Feb 3, 2022
Merged

[docs-only] Events adr #3070

merged 6 commits into from
Feb 3, 2022

Conversation

butonic
Copy link
Member

@butonic butonic commented Feb 1, 2022

writing down the decision process about events in ocis/reva

docs/ocis/adr/0015-events.md Outdated Show resolved Hide resolved
docs/ocis/adr/0015-events.md Outdated Show resolved Hide resolved
docs/ocis/adr/0015-events.md Outdated Show resolved Hide resolved
docs/ocis/adr/0015-events.md Outdated Show resolved Hide resolved
@kobergj
Copy link
Collaborator

kobergj commented Feb 1, 2022

Uh, I have 5 cents for this topic 😄

  1. Eventstore
    Do we want to store all events in a common event store? That way it would be possible to "replay" events to get the system to any state it ever was in. But this has also downsides. Most notably GDPR compliance. (Events in the store must not be deleted)

  2. "At-least-once" vs "At-most-once" QoS
    I would opt for "at-least-once". Events should be idempotent if possible. If a service goes down the events should stay in the queue until they are consumed. Otherwise a service can never be sure his data is correct

  3. Eventual consistency
    Just to mention this here: Eventual consistency brings extra complexity to the software. I personally think it is worth it but it will make it more complicated for the community to contribute.

  4. Events and multiple service instances
    Having multiple instances of the same service can lead to problems when dealing with events. For example if two services do the same action twice this could lead to (eg) multiple emails being sent. On the other hand if events are used to update in-memory caches, all service instances would need to get the event.

docs/ocis/adr/0015-events.md Outdated Show resolved Hide resolved
docs/ocis/adr/0015-events.md Outdated Show resolved Hide resolved
docs/ocis/adr/0015-events.md Outdated Show resolved Hide resolved
docs/ocis/adr/0015-events.md Show resolved Hide resolved
docs/ocis/adr/0015-events.md Outdated Show resolved Hide resolved
docs/ocis/adr/0015-events.md Outdated Show resolved Hide resolved
docs/ocis/adr/0015-events.md Outdated Show resolved Hide resolved
docs/ocis/adr/0015-events.md Outdated Show resolved Hide resolved
docs/ocis/adr/0015-events.md Show resolved Hide resolved
Klaas Freitag and others added 2 commits February 1, 2022 16:58
Co-authored-by: David Christofas <[email protected]>
Co-authored-by: David Christofas <[email protected]>
Co-authored-by: Jörn Friedrich Dreyer <[email protected]>
@micbar micbar requested review from C0rby and dragotin February 2, 2022 17:03
Copy link
Contributor

@dragotin dragotin left a comment

Choose a reason for hiding this comment

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

Good from my POV

docs/ocis/adr/0015-events.md Show resolved Hide resolved
@micbar micbar merged commit 18ca6af into master Feb 3, 2022
@delete-merged-branch delete-merged-branch bot deleted the docs-adr-events branch February 3, 2022 15:46
ownclouders pushed a commit that referenced this pull request Feb 3, 2022
Merge: c4f1c1e a718703
Author: Michael Barz <[email protected]>
Date:   Thu Feb 3 16:46:03 2022 +0100

    Merge pull request #3070 from owncloud/docs-adr-events

    [docs-only] Events adr
ownclouders pushed a commit that referenced this pull request Feb 4, 2022
Merge: c4f1c1e a718703
Author: Michael Barz <[email protected]>
Date:   Thu Feb 3 16:46:03 2022 +0100

    Merge pull request #3070 from owncloud/docs-adr-events

    [docs-only] Events adr
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.

5 participants