Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
326 stars 206 forks source link

lazy schema upgrade / data-migration for virtual objects #7407

Open warner opened 1 year ago

warner commented 1 year ago

from https://github.com/Agoric/agoric-sdk/issues/7338#issuecomment-1501000380

(note that this whole migration thing only applies to durable objects: merely-virtual objects do not outlive their incarnation, and are not ugpraded. This writeup uses "virtual objects" as a generic term, my apologies for the lack of precision)

The durable-Kind upgrade process gives authors a way to change the behavior of their virtual objects, but it does not yet have a way to change the shape of the data records during an upgrade.

@erights and @mathieu sketched out a scheme for this, and I wanted to capture it with enough detail for us to implement in the post-Vaults timeframe.

Current Implementation

In trunk today (ca. 0e49c36fc99311b49ed6de97e8eda58883f55113), each durable Kind has a descriptor vatstore key (vom.dkind.${kindID}.descriptor, once #7369/#7397 lands). This is a JSON-serialized record of { kindID, tag, stateShape }, and some information about facets.

The state of each virtual object is stored in vom.${baseref} as a JSON-serialized record of property capdata, named capdatas. This capdatas record has one property (with a matching name) for every property of the initial state data, as produced by the Kind's initializer function. The values of this record are capdata records ({ body, slots }). This means each property is marshalled separately. It also means the values are double-encoded before getting stored in the vatstore: marshal.serialize() does one JSON layer, and then the capdatas record is serialized with another layer. It also means that every vatstore value starts with an { open curly brace, since the value is always a JSON-encoded object.

Record Versions

We'll introduce a new format for the per-object state record: an array of [version, capdatas]. When loading the state of an object, we'll JSON.parse(syscall.vatstoreGet(`vom.${baseref}`), and then use typeof to see if the result is an Array (and extract the version integer), or an Object (which has an implicit version of 0). New data will always be stored as an Array, but the code will be able to handle records stored by earlier versions that used an Object.

These versions will be used as keys into the "stateShapes table". This table is stored durable in the descriptor as descriptor.stateShapes, as an array of [version, stateShapeCapData] pairs, and then held in RAM in a Map keyed by the version. The capdatas record is compressed according to the matching stateShape, so we need to know which version was used in order to deserialize correctly.

I think we'll deploy compression and alternate versions at the same time. I think we can say that any record whose capdatas is an Object is not compressed, and any record whose capdatas is an Array is compressed.

Upgrade Functions

We'll change the signature of defineDurableKind to add two new options: currentVersion and migrateData (names TBD of course).

const currentVersion = 2;
const migrateData = (oldVersion, oldData) => newData;
const makeFoo = defineDurableKind(kindHandle, initialize, behavior,
                                  { stateShape, currentVersion, migrateData });

currentVersion declares that all data records created during this incarnation will be marked with this particular version (in addition to being constrained to the current stateShape). It defaults to 0, which matches the records created by the original API.

When a method on an old durable object is invoked, and we need to build a state accessor object to pass to the behavior function, the VOM will load the state capdatas, deserialize it, and compare the record version against currentVersion. If they do not match, the deserialized oldData is passed to the migration function, which is expected to perform some type-specific upgrade/migration algorithm and return a newData object. This newData must match the current stateShape, and will be immediately serialized and written back to the vatstore (in a new record, with the updated currentVersion, so future retrievals that use the same version will not perform a migration).

The migrateData function is obligated to tolerate any previous value of currentVersion, including 0 (for records created before this API was introduced), unless the author has some reason to believe that all old records have been migrated. In the future we might introduce counters to help authors discover how many records remain of each old version, or (more likely) offline tools to scan the vatstore DB to generate these counts (since they aren't very useful to the vat itself).

currentVersion vs stateShape changes

Userspace is likely to go through multiple upgrades without changing the shape or the interpretation of the data record. Many upgrades will modify only non-Kind code, or will modify only one Kind and leave the rest alone. And when a Kind is modified, it might only be the behavior functions that are changed, and they will continue to use the same data records and stateShape constraint.

@erights raised the idea that we should track changes in stateShape to deduce transition points between schemas. A series of upgrades which use the same stateShape would all be given the same schema version.

I think it would be better to have userspace give us a distinct currentVersion value, instead of tracking stateShapes. Imagine one version which stores { balance: M.number() } and means "number of tokens, as a float (JavaScript Number)". Then, the authors realize that fixed-precision is better for balances, and 9 decimal places is sufficient, and upgrade to a second version which stores { balance: M.number() }, and means "integer number of nano-tokens, as a Nat". If we needed stateShape to change, they would also need to change the balance property name to nanoBalance or something, to artifically trigger the stateShape change sensor.

And, we need to provide an oldVersion to the migrateData function, which means we need to store a version in each data record, and the easiest way to let userspace correlate the value we store with the value migrateData gets, is to have userspace provide the value in the first place, in the form of currentVersion. And if userspace is already providing a currentVersion that must change for the sake of their future migrateData function, then the VOM should be free to act upon oldVersion !== currentVersion as well.

warner commented 1 year ago

cc @FUDCo

warner commented 1 year ago

If we implement this, we can close #7337, which is now about allowing "compatible" changes to stateShape.

warner commented 1 year ago

A few issues came up as I started to implement this:

I need to decide early whether to continue to serialize each property separately, or to switch now to marshalling all of state at the same time, to define what the [version, serializedStuff] format is.

mhofman commented 1 year ago
  • or we can never delete any of the old stateShapes, nor the objects they reference (in practice this probably isn't a big deal)

why wouldn't that be a big deal?

  • so either we must keep a counter for each version

I believe that's what the initial discussions called for

  • two-element arrays in which the first element is something other than a number (probably a record with a v: version property)

Why not do that right away ?

  • the one-per-Kind durableKindDescriptor record, which currently holds stateShape, could grow to versions: { } that describes each version, e.g. versions[0] = { stateShape }, versions[1] = { stateShape, compressed: true }

That doesn't seem backwards compatible. How can it differentiate a stateShape from this versions record?

warner commented 1 year ago
  • or we can never delete any of the old stateShapes, nor the objects they reference (in practice this probably isn't a big deal)

why wouldn't that be a big deal?

Basically, why anyone bother to put a constant vref in a stateShape? Seems like a pretty unlikely use case. Our implementation needs to tolerate it, but there's no good reason for anyone to take advantage of it. It seems way more likely for them to include an M.reference or however you express "any arbitrary vref here", rather than "specific vref here", and then it isn't a problem.

Second, if someone did do that, they were already pinning the object for the whole incarnation, so obviously they aren't worried about GCing it anytime soon. If it's such a long-lived object, then it seems unlikely that a mere upgrade is going to reduce that lifetime by much.

  • so either we must keep a counter for each version

I believe that's what the initial discussions called for

Eh.. I guess it's only a moderate hassle to implement for new versions, but at this point we have no fast way to reconstruct the version: 0 count. I suppose an intermediate position would be to treat version: 0 as pinned, and keep counters on all the subsequent versions. Is it worth the effort?

  • two-element arrays in which the first element is something other than a number (probably a record with a v: version property)

Why not do that right away ?

Because we don't need to? Because [version,statedata] is smaller than [{v: version},statedata] and this size is multiplied by the number of durable objects? I think it'd be premature generalization that also consumes more space. I only write it down to enumerate what portion of the syntax space we're claiming now, and laying down pointers for where we might choose to claim later if we need it.

  • the one-per-Kind durableKindDescriptor record, which currently holds stateShape, could grow to versions: { } that describes each version, e.g. versions[0] = { stateShape }, versions[1] = { stateShape, compressed: true }

That doesn't seem backwards compatible. How can it differentiate a stateShape from this versions record?

durableKindDescriptor is a JSON-serialized record with several properties: kindID, tag, unfaceted, facets, and (currently) stateShapeCapData. I'm talking about adding durableKindDescriptor.versions as a new one, and either removing durableKindDescriptor.stateShapeCapData or leaving it as a copy of the latest version's data.

It doesn't need to be backwards compatible: only new versions of liveslots will be parsing this data. We only need the old vatstore values to be forward compatible: new versions of liveslots need to correctly handle durableKindDescriptors left over from the original versions. We do that by saying "treat any durableKindDescriptor that lacks a .versions property as if it had a single versions[0]", something like:

  const loadDurableKindDescriptor = kindID => {
    const key = `vom.dkind.${kindID}.descriptor`;
    const raw = syscall.vatstoreGet(key);
    raw || Fail`unknown kind ID ${kindID}`;
    const parsed = JSON.parse(raw);
    if (!parsed.versions) {
      parsed.versions = { '0': { stateShape: parsed.stateShapeCapData } };
      delete parsed.stateShapeCapData;
    }
    saveDurableKindDescriptor(parsed);
    return parsed;
  };

compression

Rereading my opening comment, at the time I said "I think we'll deploy compression and alternate versions at the same time.". I no longer think that: @erights 's experiments with compression showed a serious performance hit (time spent compressing), so I don't think we should implement compression unless we think the space savings is worth the slowdown, or if we can find ways to mitigate the slowdown.

I think I like the idea of using typeof(stateCapData) === 'array'`` vsobject` as an indicator of whether the data is compressed or not. But we don't really need to make it selectable on a per-object basis: treating compression as a boolean property of each version would be fine (either all objects of a given version are compressed, or none are).

erights commented 1 year ago

There are several things about compression here that I need to clarify and/or correct. Could we fit a discussion of compression into the upcoming SwingSet kernel meeting?

warner commented 1 year ago

Our conclusions from today's kernel meeting:

And @FUDCo had a great idea: most instances don't have any slots, so let's omit all slots: [] from the recorded data. We can go even further (and this trick influences our data format), by converting { body: "XYZ", slots: [] } to just "XYZ". Something like:

const data = (typeof raw === 'string') ? { body: raw, slots: [] } : raw;
warner commented 1 year ago

I found some additional ways to reduce the size of the vatstore data, which also makes the compression more effective, but it will require a change to the way we encode things.

Current Encoding

Currently, we have the dataCache, which maps each virtual-object baseRef to two things. The first is the valueMap, which is a Map from state property name (the amount in state.amount = 123) to the actual value (e.g. 123). The second is capdatas, which is an object whose property names match those of state, and whose values are CapData records (the output of marshaller.serialize(), so an object with string .body and list-of-strings .slots). The vatstore holds a JSON stringification of capdatas. We populate capdatas once per crank, the first time anything needs state, and then we lazily unserialize the individual CapData records when something needs a particular property on state.

VOM cache lifetime - Frame 1

Then later, when someone changes the state property, the setter will immediately serialize the new value, replacing one of the values in capdatas, and also updating the value in valueMap. This is our opportunity to discover (and report) serialization problems, including attempts to store non-durables in a durable object, so it's important to do it synchronously.

VOM cache lifetime - Frame 2

We don't call vatstoreSet until the end of the crank, at which time we JSON stringify the entire capdatas object and write the resulting string to the DB

VOM cache lifetime - Frame 3

Benefits of this approach:

Drawbacks:

A further disadvantage of the current scheme is that whatever pattern-based compression we do would be limited to one state property at a time, which means we don't get to elide the property names.

As an example, if state is equivalent to { prop1: 'string', prop2: 'other' }, we will store 84 bytes into the vatstore: {"prop1":{"body":"#\"string\"","slots":[]},"prop2":{"body":"#\"other\"","slots":[]}} .

With stateShape-based compression, the "holes" in the compressed output would contain length=1 arrays like ['string'], which would expand the vatstore data by four bytes to 88: {"prop1":{"body":"#[\"string\"]","slots":[]},"prop2":{"body":"#[\"other\"]","slots":[]}}

Proposed Encoding

Instead of storing JSON.stringify(capdatas) in the vatstore, I'm thinking:

The encoding will use newlines to separate two or three fields:

${version}\n
${body}\n
${slots.join(' ')}

(and if slots are empty, omit the last line and its delimiter).

VOM cache lifetime - Frame 4

Without compression, this shrinks the vatstore data to 37 bytes: 1␊#{"prop1":"string","prop2":"other"} (note the "LF" linefeed/newline between the "1" and the "#{.."). We achieve this by omitting the "body" and "slots" names entirely: in the old encoding there were N copies of those names for N properties, but here there are zero.

With compression, it shrinks down to just 21 bytes: 1␊#["string","other"].

If the value contained a Remotable, the encoded vatstore would look like 1␊#["$0.Alleged: Brand","$1.Alleged: Purse"]␊o-12 o-23.

Benefits:

Drawbacks:

We really need to serialize at least the one modified property during the setter, to signal errors properly (immediately). We could imagine serializing only the modified property then, and wait until end-of-crank to serialize the whole record: this would remove some of the drawbacks, but raises the question of whether we update refcounts in the setter or at end-of-crank, and we'd still have duplicate serialization of the modified properties.

The wasteful serialization is more significant if we have a large number of state properties, or a mix of large/complex and small/simple properties (where we'd feel bad about a small change triggering a large serialization).

Conclusions

I think the space savings is worth the wasted marshalling calls. I need to run some experiments with mainnet data, but I think our VOs have fairly simple state.

Compression

The Pattern-based compression system that @erights built has an API that's roughly like:

const holes = compress(stateShape, value);
const capdata = marshaller.serialize(holes);

Where holes is always an Array: it contains just the non-constant parts of the value.

This means we don't strictly need to record a "compressed or not" flag in the KindDescriptor's versions table (although I think we should). The serialized non-compressed state record will always be an object, so the resulting capdata.body will always start with #{ (the # means we're using smallcaps, then the remainder is JSON parseable). But the serialized "holes" that come out of the compressor is always an Array, so its capdata.body will always start with #[.

I can't think of any good reason to allow some records (VOs) of a given version to be compressed, while allowing others to not be compressed. So I think the versions table should have a compressed field for each version. Maybe we omit it or use false when the records of that version do not use compression (including for the implicit version=0, e.g. when we upgrade the Kind for the first time we should retroactively create versions: { "0": { stateShape, compressed: false } }). But we should anticipate changing the compression scheme in the future, so maybe we should use compressed: 1 to mean "@erights 's current approach, and reserve compressed: 2 for something new in the future.

mhofman commented 1 year ago

We can go even further (and this trick influences our data format), by converting { body: "XYZ", slots: [] } to just "XYZ". Something like:

const data = (typeof raw === 'string') ? { body: raw, slots: [] } : raw;

I would like to avoid any design decision that assumes body will always be a string, or basically do not have the ability to switch encodings for CapData. See https://github.com/endojs/endo/issues/1478

mhofman commented 1 year ago
  • the vatstore data is doubly-serialized: each capdatas[propname] is the output of a marshal.serialize (which does one layer of JSON), then the whole capdatas record is again serialized (adding a second layer of JSON)

I would really like us to solve this problem more pervasively rather than ad-hoc solutions for everywhere this problems crops up. I believe that not serializing CapData when we encode it would be one solution, but there may be others

warner commented 1 year ago

I would really like us to solve this problem more pervasively rather than ad-hoc solutions for everywhere this problems crops up. I believe that not serializing CapData when we encode it would be one solution, but there may be others

I hear you, and I can imagine us making improvements in message transport, but in this saved-state case, I don't see how we can traverse the path of types from "arbitrary tree of objects" (i.e. state) to the body+slots of CapData (which are not strings) and finally reach the required vatstore value type (a string), any better than doing exactly one JSON stringification for the first step, and the ad-hoc newline thing for the second step.

What did you have in mind? We could make vatstore accept object graphs/trees, but that just hides the necessary serialization down a layer. We could change marshal to use something other than JSON in it's second pass, but then we're inventing a new JSON-alike.

For things like syscall.send, we might have other options, but even there the fact that messages get translated through c-lists and stored in queues imposes some constraints (e.g. I think the actual arguments of syscall.send() should include a CapData record, even though we might improve the subsequent encoding which transports those arguments over to the kernel process to behave better than a generic JSON.stringify). But that doesn't help us for this storage case.

mhofman commented 1 year ago

The approach detailed in https://github.com/endojs/endo/issues/1478 is to change marshal to not JSON serialize into the body, but instead just do an encoding of the passable data into a serializable #body, which means it stays an object.

That way you can nest this CapData in any other serializable structure, and only perform a single JSON.stringify when issuing the vatStore syscall (the one which already exists). The syscall handler on the kernel side can continue to JSON.parse the syscall command, and when handling the syscall, can unwrap the syscall arguments, and perform a JSON.stringify of the vatStoreSet syscall payload to store it in the DB.

Someday we'll be able to use the source addition of JSON.parse to avoid the reserialization if we're concerned about that. I am convinced however that a single parse or stringify on the vat side is a lot more efficient than the nested ones we have now.

warner commented 1 year ago

That way you can nest this CapData in any other serializable structure, and only perform a single JSON.stringify when issuing the vatStore syscall (the one which already exists). The syscall handler on the kernel side can continue to JSON.parse the syscall command, and when handling the syscall, can unwrap the syscall arguments, and perform a JSON.stringify of the vatStoreSet syscall payload to store it in the DB.

Oh, ok so I think you're changing the vatstoreSet() signature from a string value to an "arbitrary JSON-serializable" value, moving responsibility for (and the cost of) the final stringification step over to the kernel side.

Let's see.. that new type is a superset of the old "string" type, however the actual stored data would not be backwards compatible: vatstoreSet('key', 'string') used to store string, now it would store "string", and more importantly legacy storage keys with string as a value would not be decodeable by the new code, so we'd need some sort of version indicator.

(Remember that we use the vatstore for both marshalled data, in virtual-object state and virtual collections, but also for simpler things like VOM refcounts and objectID counter records).

I don't know how to achieve backwards compatibility at that layer. The kernel (or whoever is sitting closest to the DB, implementing the vatstoreGet syscall) always gets a string from the kvStore. Should it return that string directly to the vat, or should it JSON parse it first? To tolerate existing data, it must make that decision based just upon the contents. The old data was allowed to have any arbitrary string value. So I think we've already consumed all the parse space: all possible kvStore values could be generated by the old code, so there's no place to put a flag that could reliably indicate that the value was created by the new code.

FUDCo commented 11 months ago

As I was rereading this (swapping this stuff back into my head in preparation for resuming talking about acting on some of this), it struck me that we should provide a default migrateData function that implements the logic: when migrating from oldVersion to newVersion, if a property exists in oldVersion but not in newVersion, simply discard it. If it exists in newVersion but not in oldVersion, initialize it to undefined, and if it exists in both versions then throw if the shapes aren't the same. I suspect this will do for the vast majority of migrations. If we further augment this with an additional option to defineDurableKind that provides default values for properties, it could probably handle 98% of migrations with no additional code needing to be written and no need to work out complicated version A to version B to version C to version D etc migration logic. This is mostly consistent with what the SQL databases I'm familiar with already do anyway when you change a table schema.

FUDCo commented 11 months ago

Thinking further about the parallels with SQL databases, consider the ALTER TABLE statement (see, e.g., https://www.sqlite.org/lang_altertable.html) and the limitations on what it can do. A lot of the rules have to do with things like PRIMARY KEY or UNIQUE constraints, which don't really effect us. I'm wondering if with a little bit of cleverness we can achieve most of our schema migration objectives with statically declared data (in particular, by adding default values to the shape definition) rather than by requiring the user to provide migration code. Consider that developers out in the world are going to be accustomed to the kinds of constraints that SQL imposes. In particular, while it does allow a column to be renamed (which I don't think is really a thing we need to support) it does not allow a column's type to be changed. Rather, in the wild what people do is add a new column for the new type and have the code that uses the data check the old vs. new constraints in place at the point the data is used. In effect, the developer does write migration code for this one kind of case (which, in my experience is actually quite rare) but does so that the point of use rather than in some separate place. One could argue that this is better because it locates all the things that need to understand the column's meaning together in one place. In also means that you aren't having to keep track of mapping which changes are in which versions or having to worry about explicit version numbers and when to increment them.

erights commented 11 months ago

A data encoding of schema migration strategy has a further advantage: We know it the time of its application is unobservable, so we don't have to be either completely lazy or completely eager to stay deterministic. This is much like the advantages we gain when we can use a pattern as an acceptance predicate, rather than expressing a predicate in code.

mhofman commented 11 months ago

I agree a data only migration is better, and likely compatible with the expectations of JS developers to "feature test" their state.

That said, I think we should internally implement this data only migration on top of a schema version model, so that we remain compatible with a user provided migration function if the need arises.

mhofman commented 11 months ago

Would the existing patterns provide sufficient input to perform this data only migration? Is there always a default value that can be deduced from new fields added in the pattern?

erights commented 11 months ago

Not quite. But with a pattern and a copyRecord of default fields

const makeNewRecord = (oldRecord, newPattern, newDefaults) =>
  const newRecord = harden({ ...newDefaults, ...oldRecord });
  mustMatch(newRecord, newPattern);
  return newRecord;
};
warner commented 2 months ago

In the top comment (https://github.com/Agoric/agoric-sdk/issues/7407#issue-1665763948):

In the future we might introduce counters to help authors discover how many records remain of each old version, or (more likely) offline tools to scan the vatstore DB to generate these counts (since they aren't very useful to the vat itself).

and in https://github.com/Agoric/agoric-sdk/issues/7407#issuecomment-1657375743 :

so either we must keep a counter for each version (starting from zero, so we're already too late, and update it each time we add/delete/upgrade an instance)

We'll need to retain the serialized stateShape for each version until 1: it is not the current version, and 2: there are no remaining instances with state at that version. The serialized stateShape can contain references to objects (and, with compression, might be the only durable reference to an object), and that reference must be tracked with a refcount. And we can't decref until the stateShape is deleted.

In a scenario where all instances hold e.g. the same Brand, the version-0 instances will each have their own refcount for the Brand, as will the recorded stateShape, giving us a refcount of N+1. When we upgrade to version-1 and start using compression (leaving the stateShape alone), we'll start with N+2, then we'll slowly chip away at the N as we upgrade instances (on demand, as we build Representatives for them). If we're lucky and somehow touch all instances, we'll be down to just 2. And we can't get rid of the first of those until the version-0 stateShape goes away.

I'm thinking it may be useful to have counters for versions 1 and beyond, even though we won't have a (cheap) counter for version 0. It would let us delete the version-2 stateShape, some day. We could also schedule an (expensive) search for all remaining version-0 instances and forcibly upgrade them, maybe as a special vatPowers method that takes the KindHandle, or as a method on the KindHandle or something.

And I'm thinking that the counters should include { '0': 'unknown' } to explicitly mark the fact that we don't know how many version-0 instances there are. If we did do the expensive search, we'd have a place to record our success (probably by deleting the 0 property entirely, rather than retaining { '0': 0 } forever).

The cost of maintaining the counters would be an extra vatStore write for every delivery which adds, deletes, or upgrades a durable object. We might store the counters in the descriptor record (which we have to read anyways), to save a read, at the cost of the deserialization taking slightly longer. Or we might put them in a separate key.

mhofman commented 1 month ago

As an alternative or intermediate step until we implement version migration, we could instead allow partial state shapes upgrades for a subset of shape changes: existing fields must retain identical shapes (including exact identity of any remotable reference), and new fields are allowed only if they're optional (allow undefined values)

In that case the new shape would strictly be compatible with the old state data, and can thus replace the state shape in the kind description. We would need to add refcounts to any new remotables referenced by the state shape (knowing that slot numbers might have changed).

This requires a predicate to assert the "compatibility" of the state shape as described above, and changes to the state field accessors to handle possibly missing data in existing state records for these new fields.