Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.

Persistence migration between v0.31.0 and now. #1007

Open
fisx opened this issue Nov 29, 2016 · 13 comments
Open

Persistence migration between v0.31.0 and now. #1007

fisx opened this issue Nov 29, 2016 · 13 comments
Milestone

Comments

@fisx
Copy link
Collaborator

fisx commented Nov 29, 2016

Check #1001, #1002, #1004, #1005, #1006, and write migration types and SafeCopy instances so that v0.31.0 database files can be loaded into the new release.

Also check that after clean shutdown, no changelog remains that needs to be migrated as well (that would be harder to implement, and hopefully is unnecessary).

This is a meta-issue for the above and doesn't have an extra budget, but I'll open it here anyway so we won't forget.

@fisx fisx added the poldi req label Nov 29, 2016
@fisx fisx added this to the 2016-12-31 milestone Nov 29, 2016
@fisx
Copy link
Collaborator Author

fisx commented Nov 29, 2016

(With a little luck, we won't have any database changes in #1001, #1002. In any case we should move this issue to milestone 2017-03-31 when done here.)

@np
Copy link
Collaborator

np commented Dec 6, 2016

I'm pretty confident we won't need any migration \o/

@fisx
Copy link
Collaborator Author

fisx commented Dec 11, 2016

after upgrading from 0.32.0 to 0.33.0, i get this:

2016-12-11 22:48:21.180423 UTC [ERROR] openLocalStateFrom failed: Could not parse saved checkpoint due to the following error: too few bytes
From:   Persistent.Pure.AulaData:
        Types.Core.IdeaSpace:
        Types.Core.SchoolClass:
        Types.Core.ClassName:
        demandInput

I suspect that AulaData have not changed, but the transactions have? But why would a regular shutdown (via ./scripts/shutdown.sh) leave a changelog in the queue for replay? Need to investigate further.

@fisx
Copy link
Collaborator Author

fisx commented Dec 11, 2016

(perhaps kill $pid is a bit harsh?)

@fisx
Copy link
Collaborator Author

fisx commented Dec 11, 2016

No, stupid me. SchoolClass now contains a ClassName instead of an ST. So it's time to write some more safecopy types...

@fisx
Copy link
Collaborator Author

fisx commented Dec 12, 2016

The ClassName change was a mistake, I should have said no to this in the beginning. Migrating is feasible, but much more work than reverting to the old AulaData type, so I'll do the latter.

@fisx
Copy link
Collaborator Author

fisx commented Dec 12, 2016

nope, neither of them is simple. :-(

@fisx
Copy link
Collaborator Author

fisx commented Dec 12, 2016

We also need to check that shutdown via kill $pid will be caught and a checkpoint is created after the last transaction. I don't think that's currently the case, but if it's not we will run into migration issues in the serialized transaction types, not just in the content types.

@np
Copy link
Collaborator

np commented Dec 12, 2016

Since ClassName is a newtype the data representation didn't change it should be possible to make it work without any migration.

I would test this by writing an instance of SafeCopy for ClassName which simply wraps/unwraps the constructor. I can do this test if you wish.

Why do you think reverting 5a97f8f is difficult?

To deal with the kill issue we need a signal handler, however I would first check if there is already one somewhere.

@fisx
Copy link
Collaborator Author

fisx commented Dec 12, 2016

All good points.

Reverting 5a97f8f may be easy enough if you know what you're doing. (-: I got uncertain when I was changing the lenses digging into AulaData, and worried that there may be some ambiguity issues between this ST and others if I eliminate the ClassName newtype.

But writing a manual SafeCopy issue may be the best and quickest solution.

@np
Copy link
Collaborator

np commented Dec 12, 2016

I will experiment with the safecopy instance first then.

@fisx
Copy link
Collaborator Author

fisx commented Dec 12, 2016

Re. the kill $pid question, I just tried this:

@@ -24,7 +26,7 @@ withPersist' cfg mkRunP m = do
     let logger = unSendLogMsg (aulaLog (cfg ^. logging)) . LogEntry INFO . cs
     rp@(RunPersist desc _ _ close) <- mkRunP  -- initialization happens here
     logger $ "persistence: " <> desc
-    m rp `finally` close  -- closing happens here
+    m rp `finally` (trace "triggering persist close." close)  -- closing happens here

When I C-c the server, close is called. If I kill $pid, it's not.

Default signal for kill is TERM, which is bad; C-c sends INT, which is good. Fixed in #1022.

@fisx
Copy link
Collaborator Author

fisx commented Dec 12, 2016

Moving this issue to the next milestone.

@fisx fisx modified the milestones: 2017-03-31, 2016-12-31 Dec 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants