-
Notifications
You must be signed in to change notification settings - Fork 667
Store the current module hash in a system table #290
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
Conversation
|
I recommend you wait for my PR that changes and simplifies the use of system tables. |
|
I’d love to, but this one is also fairly critical for cloud to work correctly. If you haven’t considered already, I would also greatly appreciate if new system tables could be introduced after the db was bootstrapped (I.e. a gap in the reserved sequence range). |
Fill the user-retrievable database log with more info about what is going on while a database is being initialized or updated.
Add a u64 field `epoch` to the st_module table, used to store a fencing token.
Need another line of logs, as there is now more output.
Make getting and setting the program hash trait methods. Also widen the epoch / fencing token to fit stdb sequences.
| /// | ||
| /// A programmable datastore is one which has a program of some kind associated | ||
| /// with it. | ||
| pub trait Programmable: TxDatastore { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming?
This allows to actually use relational db methods taking those types as arguments outside the core crate. Also make `next_sequence` and `create_sequence` not take `&mut self` unnecessarily.
| /// Update the [`Hash`] of the program currently associated with the | ||
| /// datastore. | ||
| /// | ||
| /// The operation runs within the transactional context `tx`. The fencing | ||
| /// token `fence` must be verified to be greater than in any previous | ||
| /// invocations of this method. | ||
| fn set_program_hash(&self, tx: &mut Self::MutTxId, fence: Self::FencingToken, hash: Hash) -> Result<()>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this cause the program itself to be updated, or only the stored hash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only the hash — but inside the transaction which updates the module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a good point, actually -- the datastore doesn't store its own module. That does make sense in a distributed setting, but is another piece to worry about for keeping a replica consistent. Food for thought...
| // Release tx due to locking semantics and acquire a control db | ||
| // lock instead. | ||
| stdb.commit_tx(tx)?; | ||
| let lock = self.lock_database_instance_for_update(instance.id)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels like a race condition...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, but we can’t thread through the tx. If a later thread is faster acquiring the lock, this update will not have any effect — that is, we guarantee at most once, not exactly once.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok. Not ideal, but probably fine.
Description of Changes
Adds a new system table
st_modulewhich stores the hash of the stdb module currently associated with the database. The hash is updated transactionally when the database is initialized or updated (that is, the table does not have any rows until theinitprocedure ran successfully).st_modulealso associates an "epoch" number with the row, which is used to store a fencing token. As sketched out in the changes to standalone, this allows to use an external (distributed) locking service to safely order lifecycle operations on the database. We can thus guarantee at-most-once semantics forinitandupdatereducers.API and ABI
Because a new system table is introduced, compatibility with existing databases cannot be maintained. The patch is against
cloud-next, which contains breaking changes anyways.