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

fix: liveslots: initialize counters in startVat, not on-demand #4926

Merged
merged 1 commit into from
Mar 28, 2022

Conversation

warner
Copy link
Member

@warner warner commented Mar 26, 2022

Liveslots maintains three counters: the Export ID (used for o+NN
exported object vrefs), the Promise ID (for p+NN allocated
promises), and the Collection ID (one per virtual/durable
collection).

Originally, these counters were only held in RAM: initialized during
makeLiveSlots() and incremented when needed. To fix #4730,
makeLiveSlots() was changed to read them from the DB, and then
liveslots writes them back to the DB at the end of any delivery which
modifies them. This ensures that a future version of the vat+liveslots
can pick up allocation where the previous version left off.

However the DB read to initialize the in-RAM copy did not set the
'dirty' flag. This caused variations in DB writes across different
vats: startVat would not write out the initial values if
buildRootObject did not happen to export any additional objects. In
addition, since the DB value is a simple JSON.stringify of the
counter record, the order of the keys varied depending upon whether an
Object or Promise or Collection ID got allocated first.

This commit changes the counter initialization process to use a fixed
order for the counter names, and to always perform the
initialization (and DB write) at the same time in all vats: during
startVat, with a write at the end of the delivery. This should make
the initial kvStore contents (just after startVat) more consistent.

The new initialization code should behave correctly when we add new
counters in the future: it will merge the new counter names (and
initial values) into the record it reads from the DB during startVat.

A handful of unit tests were updated to expect the new order.

@warner warner added the SwingSet package: SwingSet label Mar 26, 2022
@warner warner requested a review from FUDCo as a code owner March 26, 2022 03:23
@warner warner self-assigned this Mar 26, 2022
@warner warner force-pushed the 1848-initialize-counters-earlier branch from 65ff42c to db1ef0a Compare March 27, 2022 00:24
Copy link
Contributor

@FUDCo FUDCo left a comment

Choose a reason for hiding this comment

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

Ser bra ut.

Liveslots maintains three counters: the Export ID (used for `o+NN`
exported object vrefs), the Promise ID (for `p+NN` allocated
promises), and the Collection ID (one per virtual/durable
collection).

Originally, these counters were only held in RAM: initialized during
`makeLiveSlots()` and incremented when needed. To fix #4730,
`makeLiveSlots()` was changed to read them from the DB, and then
liveslots writes them back to the DB at the end of any delivery which
modifies them. This ensures that a future version of the vat+liveslots
can pick up allocation where the previous version left off.

However the DB read to initialize the in-RAM copy did not set the
'dirty' flag. This caused variations in DB writes across different
vats: `startVat` would not write out the initial values if
`buildRootObject` did not happen to export any additional objects. In
addition, since the DB value is a simple `JSON.stringify` of the
counter record, the order of the keys varied depending upon whether an
Object or Promise or Collection ID got allocated first.

This commit changes the counter initialization process to use a fixed
order for the counter names, and to always perform the
initialization (and DB write) at the same time in all vats: during
`startVat`, with a write at the end of the delivery. This should make
the initial kvStore contents (just after `startVat`) more consistent.

The new initialization code should behave correctly when we add new
counters in the future: it will merge the new counter names (and
initial values) into the record it reads from the DB during startVat.

A handful of unit tests were updated to expect the new order.
@warner warner force-pushed the 1848-initialize-counters-earlier branch from db1ef0a to 700e2c2 Compare March 28, 2022 06:12
@warner warner merged commit d9d09f1 into master Mar 28, 2022
@warner warner deleted the 1848-initialize-counters-earlier branch March 28, 2022 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

exported object ID counter vs upgrade
2 participants