-
-
Notifications
You must be signed in to change notification settings - Fork 21
feat: partitioned multiple stream support #627
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
base: main
Are you sure you want to change the base?
Conversation
| } | ||
|
|
||
| public function load(string $projectionName, ?string $lastPosition, int $count, ?string $partitionKey = null): StreamPage | ||
| public function load(string $projectionName, ?string $lastPosition, int $count, ?string $partitionKey, string $streamName): StreamPage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are loading for stream and partition key. As partition key make only sense within given stream.
| return <<<SQL | ||
| CREATE TABLE IF NOT EXISTS {$this->tableName} ( | ||
| projection_name VARCHAR(255) NOT NULL, | ||
| stream_name VARCHAR(255) NOT NULL DEFAULT '', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tracking is based on stream+partition, meaning this need to be added to the projection table.
This allow for full isolation of projecting, even if the projected identifier is the same between different streams (e.g. account_id+account_stream, and account_id+wallet_stream)
| } | ||
|
|
||
| public function execute(?string $partitionKeyValue = null, bool $manualInitialization = false): void | ||
| public function execute(?string $partitionKeyValue, string $streamName, bool $manualInitialization = false): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jlabedo I do think global tracking is a bit problematic with current implementation. It makes assumption that projecting between different streams is like projecting across single stream.
I do think we should lose that up, and allow for parallel projecting between streams, just like for partitioned. This would means that large streams, while projecting with 2, 3+ streams could be 2,3+ time shorter.
However the result of that is, if we project to the same row in db between streams, we need to ensure using safe blocking SQL operations. Instead of:
- not safe: the last one wins
UPDATE count VALUES (5)- safe blocking: will give correct results in case of concurrent access
UPDATE count = count + 1I am not sure, if even projecting to same record between streams should be a case, I would discourage to do that. But if we someone would want to do it, we would have to mention it in the docs - if we go with isolated projecting between streams.
Why is this change proposed?
This provides ability for project from multiple streams for partitioned projections.
Streams just as partitions are tracked separately. This means that Ecotone provides ability to concurrent rebuild between streams.
Pull Request Contribution Terms