Skip to content

feature/dcb#61

Merged
m1l4n54v1c merged 36 commits intomasterfrom
feature/dcb
Jun 3, 2025
Merged

feature/dcb#61
m1l4n54v1c merged 36 commits intomasterfrom
feature/dcb

Conversation

@m1l4n54v1c
Copy link
Contributor

Experimental DCB API - it could be a subject of change in the future.
Retagging and Snapshotting are not yet supported by Axon Server.

saratry and others added 28 commits November 16, 2023 11:55
Refined the rest of the DCB GRPC API.
Events sent from the server will be identifiable by this reference.
Implemented first version of Intercepting Event Store Service.
Refactored the state of the API.
- Replaced Seed with Head
- Added shortcut for filtering events on multiple types
map<string, string> meta_data = 3;
int64 pendingSince = 4;
bool changePending = 5;
bool dcbContext = 6;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean you can update a context to become a DCB Context?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this is a return message from Axon Server. Just indicating whether the context is dcb or not.

string name = 1;
string replicationGroupName = 2;
map<string, string> meta_data = 3;
bool dcbContext = 4;
Copy link
Contributor

Choose a reason for hiding this comment

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

If I am not mistaken, the "meta_data" field here is rather configuration for the context, right? Couldn't the fact that it is a DCB context be part of that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

No preference here. Having it as a separate field makes it more explicite, but it may be redundant in the future.

/* Gets the current _tail_ of the Event Store. The _tail_ points to a sequence of the first event stored. */
rpc GetTail (GetTailRequest) returns (GetTailResponse);

/* Gets the sequence of the event whose timestamp approximately matches the given request. It returns the first
Copy link
Contributor

Choose a reason for hiding this comment

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

"Approximately match" is very vague. I suggest:

Returns the lowest sequence of an event with a timestamp equal to or higher than the given timestamp. The HEAD is returned if no events exist with a timestamp equal to or higher than the given timestamp.

The NB part is good. Best to warn people that the timestamps are application-provided and that Axon Server has no control over them. Note that "monotonically" is not relevant here. Monotonically means that the delta between each event's timestamp is equal. I would replace that with "strictly".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with the first part.

Formal Definition of Monotonically Increasing Function:
A function f(x) is monotonically increasing if for all x1 and x2 in its domain where x1 ≤ x2, then f(x1) ≤ f(x2). 

The values must be greater than or equal to. It says nothing about deltas.

Copy link
Contributor

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

Since I am already using this API in AF5, I am fine with what it looks like. I only have some documentation nits here and there.

There are two exceptions though:

  1. What about using version instead of revision for the snapshots?
  2. Why use key instead of something like identifier?

However, those two exceptions are on the snapshot API, which you pointed out is still to be finalized. Henceforth, I am approving this PR.

string name = 1;

/* The revision of the snapshot. */
string revision = 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: we have started to move away from revision, in favor of version, within Axon Framework. Although it's up to the Server team in the end, we figured it would be clearer to simply use version, as the word revision raises some eye brows from time to time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Granted, this part of the API is still up for change as you pointed out. So I don't mind if this is taken care off later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll leave it for now as is since it's prone to change.

int64 sequence = 2;

/* If set to true, older snapshots for the same key are pruned. */
bool prune = 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ow nice, I like this!

m1l4n54v1c and others added 8 commits May 15, 2025 16:34
Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
Copy link
Contributor

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

Still think this looks fine.

@m1l4n54v1c m1l4n54v1c merged commit ef64726 into master Jun 3, 2025
1 check failed
@m1l4n54v1c m1l4n54v1c deleted the feature/dcb branch June 3, 2025 09:17
@smcvb
Copy link
Contributor

smcvb commented Jun 3, 2025

🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants