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

[DB-26-19] Warn when projection state becomes greater than 8mb #4276

Conversation

lakshdeepsingheventstore
Copy link
Contributor

@lakshdeepsingheventstore lakshdeepsingheventstore commented May 30, 2024

Added : A warning to the log when a projection state size becomes greater than 8 MB. https://eventstore.aha.io/develop/requirements/DB-26-19

@lakshdeepsingheventstore lakshdeepsingheventstore force-pushed the lakshdeepsingh/warn-when-projection-state-is-getting-too-big branch from 8d6fbc5 to 4e4e2ab Compare May 30, 2024 17:05
Copy link
Contributor

@timothycoleman timothycoleman left a comment

Choose a reason for hiding this comment

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

  • in the pr description, this isn't a bug so we don't need to put fixed. we can use Added:
  • im not sure, but does this apply to projection state specifically or all emitted projection events?

Copy link
Contributor

@timothycoleman timothycoleman left a comment

Choose a reason for hiding this comment

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

^

@lakshdeepsingheventstore lakshdeepsingheventstore force-pushed the lakshdeepsingh/warn-when-projection-state-is-getting-too-big branch from 82fff3e to 9aaf629 Compare June 10, 2024 15:02
Copy link
Member

@hayley-jean hayley-jean left a comment

Choose a reason for hiding this comment

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

Thanks! There's one more place where the state size needs to be checked

What you have here works for the projection result that will be written to the $projection-{projection_name}-result stream. This result is only written for Queries, or for Continuous/OneTime projections that call outputState() in the projection definition.

The size of the state that makes up the checkpoints written to the $projection-{projection_name}-checkpoint stream and $projection-{projection_name}-{partition_name}-checkpoint streams also needs to be checked.

Probably the best place to check and log a warning for the state size is when StateUpdated is called in the CoreProjectionCheckpointManager (in addition to the check you're doing here in ResultEventEmitter)

@lakshdeepsingheventstore
Copy link
Contributor Author

lakshdeepsingheventstore commented Jun 17, 2024

The size of the state that makes up the checkpoints written to the $projection-{projection_name}-checkpoint stream and $projection-{projection_name}-{partition_name}-checkpoint streams also needs to be checked.

Making changes to CoreProjectionCheckpointManager solves the same purpose as was being done by the changes in ResultEventEmitter file. So in the latest commit/changes, I have removed the changes from ResultEventEmitter, and added those in CoreProjectionCheckpointManager in the method StateUpdated (this would make it more clear that the state size is being checked)
However, to exclusively check for state sizes for the checkpoint streams, separate checks had to be added to CoreProjectionCheckpointWriter and PartitionStateUpdateManager files, since the check in CoreProjectionCheckpointManager does not log anything when the projection Checkpoint is written.

Copy link
Member

@hayley-jean hayley-jean left a comment

Choose a reason for hiding this comment

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

Sorry about not including this in my earlier review, but I learned new things about how the result of a projection interacts with the state while testing this. Basically, we need to include both the state and result in PartitionState when checking the size of the projection state.

I previously thought that the result was only written if you had outputState() defined on your projection, but it turns out that the check for that is commented out.

Instead, a result is always written for continuous projections (just not always in a result stream). If the projection includes a transformBy() function then the result will be the result of that transformation run on the state. Otherwise it simply duplicates the state into the result.

The result is included with the state when the checkpoint is written, so this can effectively double the state size that is written to disk.

I think the best way to handle this would be to have a Size property on PartitionState that returns something like _state.Length + _result?.Length ?? 0; and use that for the warning log. Otherwise, by the time the warning is logged at 8mb, you are already potentially over the 16mb limit.

Let me know if you want a sample projection to test this case

…hecks and mew size property added in Partition State
@lakshdeepsingheventstore lakshdeepsingheventstore force-pushed the lakshdeepsingh/warn-when-projection-state-is-getting-too-big branch from 2499740 to 6c61623 Compare June 24, 2024 23:15
@hayley-jean hayley-jean self-requested a review June 25, 2024 09:42
@hayley-jean hayley-jean merged commit 1cad4e7 into master Jun 25, 2024
8 checks passed
@hayley-jean hayley-jean deleted the lakshdeepsingh/warn-when-projection-state-is-getting-too-big branch June 25, 2024 10:00
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.

None yet

3 participants