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

Confusion between 1-based and 0-based stream positions in different DB implementations of Event/Checkpoint Store #163

Open
ReinderReinders opened this issue Nov 28, 2022 · 8 comments

Comments

@ReinderReinders
Copy link
Contributor

I am working on a Proof-Of-Concept using Eventuous with PostgresDB as the Event Store, and Microsoft SQL Server as the Checkpoint Store / read model store.
I noticed the following confusing issue:

the global_position in the Postgres Schema is maintained as a 1-leading array:

image

However the Subscription in the receiving SQL Server store tracks the same global stream as a 0-leading array:

image

I have double-checked and the Subscription tracks in real-time, is in sync, no event was lost or skipped etc., my 'receiving' application is not stalling. Triggered one more event after above screenshots to demonstrate this:

[Postgres Event Store]
image
[SQL Server Checkpoint Store]
image

I find this confusing and not very intuitive. Am I missing some part of the implementation / intention here, or is this really just a little bug?

Kind regards,
Reinder

@alexeyzimarev
Copy link
Contributor

I am not sure why that is since the checkpoint should be the global position value of the last processed event.

I would not expect the checkpoint store to manipulate the value in any way. It might be that the stored procedure that reads events from Postgres is doing something. Can you guess where the issue is coming from?

@ReinderReinders
Copy link
Contributor Author

ReinderReinders commented Nov 28, 2022

I'd guess maybe in the Eventuous.SqlServer.Subscriptions namespace, SqlServerAllStreamSubscription, AsContext method:

image

Looks like a hardcoded conversion from 1-leading to 0-leading to me?

edit: this implementation is identical to the Postgres Subscription implementation. I can't remember seeing this issue there before.. I'll run a test.

edit 2: The behaviour is identical (i.e. offset by one) using Postgres as the Checkpoint store:

image

@alexeyzimarev
Copy link
Contributor

Yeah, I can't remember now. I was cleaning up the Postgres version, and it was taken as a baseline for the MS SQL version. Indeed, we'd better confirm what's where. My guess is the global position number is produced by the sequence (Postgres) or identity enumerator in SQL Server, and it always starts with one, not zero. As we want to keep the first event as zero position, it requires such manipulation. So, inside the system, it is consistent (zero-leading).

@ReinderReinders
Copy link
Contributor Author

ReinderReinders commented Nov 29, 2022

Yes, you were right on the money. Postgres does not accept a 0-based sequence:

image

(I had hoped this would work but it is a technical impossibility: )

image

Edit: Makes me wonder though, why should we want the first event as the zero position? This causes another issue that I will post shortly.
Edit2: This is the issue I am referring to: #164

@alexeyzimarev
Copy link
Contributor

That's what I meant. So, basically, the + 1 in the context construction just brings the sequencing to zero-lead as it's the default behaviour for Eventuous. It's confusing when you look at the database. For the rest - not so much. Maybe just mentioning it in the docs would suffice?

@alexeyzimarev
Copy link
Contributor

With #164 presumably fixed, is this one still relevant?

@ReinderReinders
Copy link
Contributor Author

@alexeyzimarev this issue is not specifically a bug, more like a feature :) I anticipate a hard time explaining to my colleagues why there is an offset between the global_position and the checkpoint position. But it is not breaking in any way so I guess we can live with it. It's just that I expect it to be confusing for (beginning) users of Eventuous. Adding a note to the documentation is of course always a good idea.
I understand the need to keep existing functionality intact (since Eventuous is already being used in production by others as far as I know), so this issue should not be corrected, but I was thinking we might make it configurable (making checkpoints 0 or 1 based I mean)? With the default value being the way it is now. That way users can choose their own preferred implementation.
But otherwise I would be all right with closing this issue.

@alexeyzimarev
Copy link
Contributor

alexeyzimarev commented Jan 5, 2023

Yeah, it's a bit too late as changing the sequence would produce hidden consequences of skipping one event. I haven't been able to document both SQL stores yet, will keep in mind this issue.

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

No branches or pull requests

2 participants