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

[Draft] Sorted partition ids alphabetically in EventStore #23

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JBBianchi
Copy link
Member

Important note:
This fix only applies to fresh installs.
For people with an existing setup, they either need to drop their EventStore volume or edit/reset the partitions:

  • cloud-events-partition-ids-BySource
  • cloud-events-partition-ids-BySubject
  • cloud-events-partition-ids-ByType

by adding stream.sort();

fromStream('cloud-events')
    .when({
        $init: () => [],
        $any: (stream, evt) => {
            if (!evt || !evt.metadataRaw || !evt.metadataRaw) return;
            const metadata = JSON.parse(evt.metadataRaw);
            const id = metadata.##propertyName##;
            if (!id || stream.includes(id)) return;
            stream.push(id);
+           stream.sort();
        }
    });

Closes #20

@JBBianchi JBBianchi added the enhancement New feature or request label Mar 24, 2023
@JBBianchi JBBianchi self-assigned this Mar 24, 2023
@cdavernas
Copy link
Member

Cant run it here, but isn't your example just gonna push the event to the "cloud-event" stream (ie. stream), then ordering it? This is obviously not the expected behavior

Copy link
Member

@cdavernas cdavernas left a comment

Choose a reason for hiding this comment

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

Can you fix the warnings in Store? You need to either to a null check, or add an exclamation mark at the end of line 278

@JBBianchi
Copy link
Member Author

Cant run it here, but isn't your example just gonna push the event to the "cloud-event" stream (ie. stream), then ordering it? This is obviously not the expected behavior

Not sure to understand what you mean. It's the same projection than before except now, the partition content is sorted each time its modified instead of just being modified. Does that make sense ?

@JBBianchi JBBianchi changed the title Sorted partition ids alphabetically in EventStore [Draft] Sorted partition ids alphabetically in EventStore Mar 24, 2023
@JBBianchi
Copy link
Member Author

This PR raises a concern:

Should we reorder the ids in the ES projection or only when read/required.

If it's sorted directly in the projection, then we loose a piece of information: the order the partition were created. For instance, it won't be possible to get the 10 last created subjects.

If not, then it means "computing" for each read which adds a little overhead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sort partition ids
2 participants