-
Notifications
You must be signed in to change notification settings - Fork 23
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
Traditional Begin and Commit transactions support in websockets #36
Comments
On a different subject, shouldn't startbase be a npm library? export class MyPersonalDO extends StartBase {
async myBusinessLogic(...args) {
this.query("select 1")
// ...
}
} |
@G4brym Great suggestions! First of all I loved seeing your project early on when the SQLite announcement came for Durable Objects!
I'll be honest, I had no idea. Out of curiosity and to edify myself, how does Django use the BEGIN/COMMIT/ROLLBACK paradigm?
Question here and perhaps you know better than I do. I am in the middle of refactoring how Starbase positions logical components in the Worker versus the Durable Object and moving some of the web sockets logic away from the DO and into the Worker (see blog post here talking through it a bit: https://starbasedb.com/blog/rethinking-the-starbasedb-architecture/). In doing this, and hopefully I'm wrong, but it seems like the Tags API isn't available for web socket connections from a Worker? I had to go about it like this: clientConnected() {
const webSocketPair = new WebSocketPair();
const [client, server] = Object.values(webSocketPair);
server.accept();
server.addEventListener('message', event => {
const { sql, params, action } = JSON.parse(event.data as string);
if (action === 'query') {
const executeQueryWrapper = async () => {
const response = await executeQuery(sql, params, false, this.dataSource);
server.send(JSON.stringify(response));
};
executeQueryWrapper();
}
});
return new Response(null, { status: 101, webSocket: client });
}
Question here. Could a bad client side implementation then that never passes a second message for
Yes! Matter of fact there is someone contributing a WIP pull request that does this and allows for it to be easier to implement into other projects. See here: #29 |
I went ahead and made a work in progress pull request on my current line of thinking on approach. Would be curious if what you're proposing here would still work with that approach or not (especially given that I'm thinking tags API isn't available in Workers but only Durable Objects – maybe I'm wrong though?). |
Django always opens a new transaction while applying a new migration, so if anything goes wrong, while moving data between tables, or adding columns, there is always a safe point to recover
looking at the docs, it does seem to be only available inside DO's
I think websocket workers, can be evicted just like a normal worker. So if a client is connected to it instead of a DO, the connection can be evicted midway through the transaction
Totally forgot to mention that, but in my implementation, i added a check on the async webSocketClose(ws, code, reason, wasClean) {
ws.close(code, "Durable Object is closing WebSocket");
// If the socket disconnecting is the one in power, rollback!
if (this.isLocked()) {
const tags = this.ctx.getTags(ws)
if (tags[0] === this.sessionIdInPower) {
await this.rollbackTransaction()
}
}
} But a max time since last message, sounds great! |
I will take a look in a few days 😄 |
Is your feature request related to a problem? Please describe.
Traditional frameworks like Django rely heavily on sql transactions, like
BEGIN;
COMMIT;
andROLLBACK
.Recently, I've created a Django library called django-cf that allows developers to connect Django apps to both D1 HTTP API or to a Durable Object DB via another small project I've made.
In that project I've already implemented the "traditional" begin and commit functionality as I will be describing in this issue, and it works quite well and fast.
So I think it makes sense to bring such a feature to startbase 😄
This feature will allow startbase to grow even more, as this will open the door to other traditional frameworks to write startbase adapters to laravel, rails, and other. Django will already have a working driver, that i wrote.
Describe the solution you'd like
This feature involves modifying 2 key parts of the websocket implementation:
First, when a new connection is made, we must generate a unique id, usually uuid, and connecting this id with the socket using the tags api.
After this we will be able to identify the socket that is sending messages when a new one arrives.
Currently, startbase websocket messages, only expects messages with
action=query
I'm proposing adding 3 more
actions
to the websocket implementation, calledtransaction.begin
,transaction.commit
andtransaction.rollback
When the server receives a
transaction.begin
, we get the socket unique id, generated in the previous step and save that in a local variable inside the DO, as var 1 (name to decide).We also record the current PITR into another variable locally, as var 2 (name to decide).
Note that both these variables must not be saved in the DO KV, because if the DO resets for some reason, the transaction status must be cleared.
From this point on, a new transaction is taking place, so when a server receives a message with action=query, we must check if the message is from the socket that sent the transaction.begin.
If this is true, we execute the query, otherwise, we must block the query, this can be done using timeouts, this way the query execution will just be delayed, until the transaction is cleared.
If the server receives a
transaction.commit
, we clear the var 1 and 2.Clearing these variable, will allow other sockets that have sent queries while the DO was blocked, to start executing automatically.
If the server receives a
transaction.rollback
, we just need to rollback the DO to the recorded PITR saved in var 2 and trigger a object reset for the DO to be restarted.Example implementation here
There are some small differences from the implementation described above, and the one in the link, but are very small, and i think the one described is better.
Additional context
I'm willing to make the pull request, I just want to get the project maintainers opinion and approval on the implementation, before starting
The text was updated successfully, but these errors were encountered: