-
Notifications
You must be signed in to change notification settings - Fork 11
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
Update main README.md #197
Conversation
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.
Left some questions for you @Arkatufus
|Recovering8 | 75466|65515| 63960|84.75%|97.63%| | ||
|RecoveringFour | 59259|51355| 58437|98.61%|113.79%| | ||
|RecoveringTwo | 41745|35512| 41108|98.47%|115.76%| | ||
| Test | SqlServer | SqlServer Batching | Linq2Db | vs Normal | vs Batching | |
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 new performance numbers look great - what's responsible for those improvements? Akka.NET v1.5, .NET 7, or a combination of factors?
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.
I really have no idea, all I did was re-run the benchmark on the latest SqlServer and Linq2Db version
| Recovering | 60516 | 77688 | 64457 | 106.51% | 82.96% | | ||
| Recovering8 | 116401 | 101549 | 103463 | 88.89% | 101.88% | | ||
| RecoveringFour | 86107 | 73218 | 66512 | 77.24% | 90.84% | | ||
| RecoveringTwo | 60730 | 53062 | 43325 | 71.34% | 81.65% | | ||
|
||
## Sql.Common Compatibility modes |
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.
I'd say "Akka.Persistence.Sql.Common` here, just to be explicit.
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.
It is measured using SqlServer though, we should consider that as a possible variable, there might be optimization at the SQL generator level that might affect benchmarks
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.
My comment pertains to the "Sql.Common Compatibility modes" header, not the benchmarks
journal { | ||
plugin = "akka.persistence.journal.sql" |
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.
Isn't this setting definitely needed in order to get Akka.Persistence to load the plugin correctly?
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.
No, this is a dump of the default hocon configuration. That line should NOT be in the default configuration file.
It is in the Setup
section on line 23-38.
```hocon | ||
akka.persistence { | ||
snapshot-store { | ||
plugin = "akka.persistence.snapshot-store.sql" |
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.
Again, isn't this setting needed in order to get this plugin to load correctly?
Fixes #179
Fixes #180
Changes