liqd / aula

An online platform for political participation in schools in Germany (not in active development)
https://liqd.net
Other
27 stars 5 forks source link

Persistence migration between v0.31.0 and now. #1007

Open fisx opened 7 years ago

fisx commented 7 years ago

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 commented 7 years ago

(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 commented 7 years ago

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

fisx commented 7 years ago

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 commented 7 years ago

(perhaps kill $pid is a bit harsh?)

fisx commented 7 years ago

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

fisx commented 7 years ago

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 commented 7 years ago

nope, neither of them is simple. :-(

fisx commented 7 years ago

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 commented 7 years ago

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 5a97f8fdaa280eaf9ee8996880658bff0ce7d86f 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 commented 7 years ago

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 commented 7 years ago

I will experiment with the safecopy instance first then.

fisx commented 7 years ago

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 commented 7 years ago

Moving this issue to the next milestone.