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

Query mechanism for Akka.Persistence SQL-based journals #1306

Merged
merged 1 commit into from
Oct 31, 2015

Conversation

Horusiath
Copy link
Contributor

WARNING: this PR contains breaking changes for SQL-based plugins. After merging, changes should be applied in dependent sql plugins. Querying mechanism should also be documented.

Changes regarding Akka.Persistence

  • All Akka.Persistence messages from now implement structural equality and meaningfull ToString messages.
  • Some of the message constructors have been hardened with requirement for not-null arguments.
  • All Persistence specs use xUnit2 ITestOutputHelper for spec output logging.
  • More meaningfull error messages, when journal/snapshot plugin couldn't be loaded because dependent module it's not linked by user project.

Changes regarding Akka.Persistence.Sql.Common

  • Querying mechanism - now user can query the journal using Query message. Message may contain set o IHint instances used to filter the data (currently only Manifest field and collection of PersistenceIds are supported, more to be written in the future). Response is collection of QueryResponse messages containing events filtered for query followed by QuerySuccess message which finishes response transmission. If an error occurred final message send is QueryFailure message
  • (breaking change) Database column PayloadType has been renamed to Manifest. Now it's more general and can contain user defined manifests if necessary.
  • SQL-based journals can now use common abstraction in form of SqlJournal class. This simplifies further plugin development and handles query messages.
  • Attached new project Akka.Persistence.Sql.Common.TestKit that contains SqlJournalQuerySpec to be inherited by dependent plugins specs in order to verify if querying is working correctly.
  • Extracting connection string from either HOCON or App.config is now handled at this library level.

Changes regarding Akka.Persistence.SqlServer

  • Akka.Persistence.SqlServer is again joined into core repo - reason for that is that Akka.Persistence.Sql.Common contained dead code, that was not used/tested anywhere in core solution. Me and @rogeralsing agreed that at least one sub-plugin should be presented here.
  • Plugin has been updated to use features from the current Akka.Persistence.Sql.Common lib.
  • Extension will now log at Warning level info, if used connection string don't specify MultipleActiveResultSets=True, as it may be reason of future problems. /cc Multiple active result sets exception #1291
  • Journal will also log at Error level if after insertion command will return result other than 1 for any reason (which is not a desired behavior).

@Aaronontheweb
Copy link
Member

Weird... why didn't the build server pick this up?

@Aaronontheweb
Copy link
Member

Ah, figured it out - the timestamp of this commit is from August 18th, so TC won't pick it up.

Could do the following and update the timestamp to present time? http://stackoverflow.com/questions/454734/how-can-one-change-the-timestamp-of-an-old-commit-in-git

@Horusiath
Copy link
Contributor Author

@Aaronontheweb all tests except those, which require SQL Server to be initialized, have passed.

@Horusiath
Copy link
Contributor Author

I've introduced few more changes:

  • No more DbConnection caching - now it's created from scratch for each operation.
  • SnapshotStore delete methods are now async (to reflect incomming changes on akka 2.4).
  • Whole sql persistence API has been remade with asyn/await in mind.

This changes also addresses issues #1327 and #1328.

@rogeralsing
Copy link
Contributor

error occurred while establishing a connection to SQL Server. The server was not found or was not accessible. Verify that the instance name is correct and that SQL Server is configured to allow remote connections.

Motivation:

One of the popular problems related to Akka.Persistence is support for aggregation of events across multiple persisted streams. Since partitioned journals are part of the Akka.Persistence plugin design, this is hard to achieve using generic tools. Instead each journal provider should be allowed to define set of querying operations, it supports.

Purpose of this commit is to introduce common set of querying messages to be used by all SQL-backed journals. This has been provided in form of Query message, which can take a collection of IHint parameters, that are used to specify filters around queried events. Currently there are three of them possible:

- Query by provided persistence id collection.
- Query by provided message manifest - manifest is often correlated with message type, but in this case it's won't take a inheritance rules when filtering a message.
- Query by message timestamp range - range can be one-side open or closed one. Range is always left-side inclusive (From) and right-side exclusive (To) in order to avoid resending the same events in sliding window scenarios.

In result, journal will emit each IPersistentRepresentation containing an event wrapped in QueryResponse message. End of the transmission will be signaled by QuerySuccess message and any possible errors will cause QueryFailure to used as a reply.

Additional changes:

In addition this commit fixes issue with concurrent access to SQL transaction by creating a dedicated sql connection object for each request.

BREAKING CHANGES:

The biggest change is modification of existing SQL table schemas - 'payload_type' field has been renamed to 'manifest' and aditional 'timestamp' field has been added.

Another change is a generic 'SqlJournal' class that can be used as base class for incomming journal implementations for any future specific SQL providers. Also dependent types (like 'JournalDbEngine' and 'IQueryBuilder') have been changed to make querying possible.
@Horusiath
Copy link
Contributor Author

@rogeralsing @Aaronontheweb Builds are passing. Could you review the changes?

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