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

store event data in separate column, no wrapping, #12 #21

Merged
merged 1 commit into from
Feb 16, 2016

Conversation

patriknw
Copy link
Member

Not ready, just a starting point for discussion.

@patriknw patriknw mentioned this pull request Jan 20, 2016
row.getInt("ser_id"),
row.getString("ser_manifest")
).get
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I just duplicated these to 3 places, needs some DRY

@magnusart
Copy link
Contributor

Moved comment here:

@patriknw I had a quick look and started thinking about how to verify that it really works. I'd really like to have a test for it since it would be very time consuming to go back and forth when debugging issues related to the migration if I have to do the migration manually each time.

I'm considering the following approach: take one of the snapshot tests and pause it just before it exits, export the data from Cassandra tables and use that as a base for a DB-script that would populate the Cassandra test database at the beginning of the migration test and the proceed to test migration and persisting new events.

Would that cover it? suggestions welcome!

The only extra scenario I can come up with is that the user moves back and forth between versions after recording data. But I cannot judge yet if that would make any difference, are we forwards compatible as well?

@magnusart
Copy link
Contributor

As far as I can see there is no actual schema migration taking place yet. I think the old create table should stay and an additional alter table should add the new columns instead. Like a poor mans evolution script (except no going back). Sound OK?

FYI: I had a look at krasserm/akka-persistence-cassandra#64 to see if I could solve that issue at the same time during schema migration. But contrary to what is stated in the README that migrating from 0.3x to 0.4x will be solved, I don't think that it will be feasible to do that within the scope of this project. The old table used the compact storage setting (as mentioned by asnare) which prevents new columns to be added except as part of the primary key.

@patriknw
Copy link
Member Author

I would have expected a one time manual test of the backwards compatibility, but if you think you can pull off an automated test I will not complain (as long as it is not too complicated to maintain).

We don't have to care about forward compatibility. We try to be nice about backwards compatibility, i.e. reading data stored with old version. Right now we are only doing it on best effort basis, and some changes will require data migration (ETL) by external tools. Once we have released 1.0 we will be more strict.

I'm not sure I understand the implications of keeping old create table + alter table. I was thinking that alter table should just be documented in migration guide. Regarding the materialized view I think it has to be dropped and re-created.

@magnusart
Copy link
Contributor

Aha, you were thinking that the user should manually migrate their schema?

I was assuming an automated migration process (perhaps a bit colored after reading the krasserm/akka-persistence-cassandra#64 which talks about automated schema migration). So in that case the user will have to manually apply the schema changes when migrating.

About the tests, at least I'll give it a try and you can have a look. I hope it will not be more work than creating a throw away application just for the purpose of testing backwards compatibility.

* The event data, snapshot data and meta data are stored in a separate
  columns instead of being wrapped in blob.
* This makes it easier to browse the event data with standard tools.
@patriknw patriknw force-pushed the wip-12-browsable-patriknw branch from 6046489 to f40e251 Compare February 14, 2016 19:44
@patriknw patriknw changed the title first attempt at storing event instead of PersistentRepr store event data in separate column, no wrapping, #12 Feb 14, 2016
@patriknw
Copy link
Member Author

@magnusart I have updated this. Did similar for snapshots. Tested migration manually, and added migration guide.

@patriknw patriknw closed this Feb 15, 2016
@patriknw patriknw reopened this Feb 15, 2016
@pjan
Copy link
Contributor

pjan commented Feb 16, 2016

@patriknw is there a schedule for this? I know there is a migration path, but if possible would prefer to avoid doing that and try timing an internal release to production based on this.

@patriknw
Copy link
Member Author

@pjan I will release a new version as soon as Akka 2.4.2 is released, and then this will be included. That should be tomorrow, or within a few days.

@patriknw patriknw force-pushed the wip-12-browsable-patriknw branch from 9a7525f to f40e251 Compare February 16, 2016 19:07
@patriknw
Copy link
Member Author

interesting, JournalPerfSpec failed several times when running on Travis when I tried to use cassandra-all 3.0.3 (worked fine locally), reverted back to 3.0.2 for now

patriknw added a commit that referenced this pull request Feb 16, 2016
store event data in separate column, no wrapping, #12
@patriknw patriknw merged commit 9c5aef2 into master Feb 16, 2016
@patriknw patriknw deleted the wip-12-browsable-patriknw branch February 16, 2016 19:39
@magnusart
Copy link
Contributor

@patriknw Very cool. Need to give this a spin now.

@patriknw
Copy link
Member Author

@pjan v0.10 is released

@pjan
Copy link
Contributor

pjan commented Feb 17, 2016

@patriknw thank you so much for this. Akka team is nothing short of amazing.

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.

3 participants