Skip to content

Optimize aggregate read operation#892

Merged
dmdashenkov merged 16 commits intomasterfrom
store-events-since-snapshot-in-mem
Dec 11, 2018
Merged

Optimize aggregate read operation#892
dmdashenkov merged 16 commits intomasterfrom
store-events-since-snapshot-in-mem

Conversation

@dmdashenkov
Copy link
Contributor

@dmdashenkov dmdashenkov commented Nov 26, 2018

In this PR we make the AggregateRepository read the required amount of history events, even if this number is greater than the snapshot trigger.

Previously, the number of read events was always equal to the snapshot trigger.

Also, now we cache the eventCountAfterLastSnapshot in the Aggregate instance. This helps us not to re-fetch the value when appending the aggregate history.

@dmdashenkov dmdashenkov self-assigned this Nov 26, 2018
@dmdashenkov dmdashenkov requested a review from armiol November 26, 2018 18:30
@dmdashenkov
Copy link
Contributor Author

@armiol, PTAL.

@codecov
Copy link

codecov bot commented Nov 26, 2018

Codecov Report

Merging #892 into master will increase coverage by <.01%.
The diff coverage is 93.33%.

@@             Coverage Diff             @@
##             master    #892      +/-   ##
===========================================
+ Coverage     93.39%   93.4%   +<.01%     
- Complexity     3868    3870       +2     
===========================================
  Files           529     529              
  Lines         12627   12637      +10     
  Branches        704     704              
===========================================
+ Hits          11793   11803      +10     
+ Misses          606     605       -1     
- Partials        228     229       +1

@dmdashenkov dmdashenkov changed the title Remove redundant DB calls Optimize aggregate read operation Dec 7, 2018
@dmdashenkov
Copy link
Contributor Author

@armiol, PTAL.

Copy link
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

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

@dmdashenkov let's discuss the naming. Other than that, LGTM — once, hopefully, the build passes.

*
* @see AggregateStorage#readEventCountAfterLastSnapshot(Object)
*/
private int persistedEventCount = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

To me, the name of the variable does not correspond to its description and usage.

@dmdashenkov
Copy link
Contributor Author

@armiol, PTAL.

@SpineEventEngine SpineEventEngine deleted a comment Dec 8, 2018
@dmdashenkov dmdashenkov merged commit ef6c949 into master Dec 11, 2018
@dmdashenkov dmdashenkov deleted the store-events-since-snapshot-in-mem branch December 11, 2018 10:30
@armiol armiol mentioned this pull request Dec 20, 2018
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.

2 participants