-
Notifications
You must be signed in to change notification settings - Fork 6
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
indexer: add kv plugin #84
Conversation
packages/indexer/src/plugins/kv.ts
Outdated
}); | ||
|
||
indexer.hooks.hook("handler:exception", async () => { | ||
await db.exec("COMMIT TRANSACTION"); |
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.
My Bad, This will be ROLLBACK TRANSACTION
. i will update it tomorrow morning.
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.
I think it's a promising start! I think we need to add tests to the KVStore
by using the in memory sqlite database.
"handler:after": ({ output }: { output: TRet[] }) => void; | ||
"handler:exception": ({ error }: { error: Error }) => void; |
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.
Is this handler called anywhere? I think we should catch the handler exception and then rethrow it after calling the hook.
packages/indexer/src/plugins/kv.ts
Outdated
private _endCursor: Cursor, | ||
) {} | ||
|
||
async get<T extends Record<string, unknown>>(key: string): Promise<T> { |
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.
I think here T
can be anything that is json encodeable (e.g. I often store just a number or a string).
packages/indexer/src/plugins/kv.ts
Outdated
return row ? deserialize(row.v) : undefined; | ||
} | ||
|
||
async put<T extends Record<string, unknown>>(key: string, value: T) { |
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.
Same as above.
packages/indexer/src/plugins/kv.ts
Outdated
await this._db.run( | ||
` | ||
UPDATE kvs | ||
SET to_block = ? | ||
WHERE k = ? AND to_block IS NULL | ||
`, | ||
[Number(this._endCursor.orderKey), key], | ||
); | ||
|
||
await this._db.run( | ||
` | ||
INSERT INTO kvs (from_block, to_block, k, v) | ||
VALUES (?, NULL, ?, ?) | ||
`, | ||
[Number(this._endCursor.orderKey), key, serialize(value)], | ||
); |
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.
I'm not sure how this will behave if the user inserts the same key
twice in the same block. We need to add tests to know for sure.
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.
A couple of small fixes and it's ready.
await db.exec(` | ||
CREATE TABLE IF NOT EXISTS kvs ( | ||
from_block INTEGER NOT NULL, | ||
to_block INTEGER, | ||
k TEXT NOT NULL, | ||
v BLOB NOT NULL, | ||
PRIMARY KEY (from_block, k) | ||
); | ||
`); |
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.
Maybe put this logic in a static initialize(db)
method on the KVStore
? That way we can share the logic between tests and hooks.
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.
Sorry I should have caught it in the previous review round!
packages/indexer/src/plugins/kv.ts
Outdated
ctx.kv = new KVStore(db, finality, endCursor); | ||
|
||
await db.exec("BEGIN TRANSACTION"); |
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.
My advice is to put this into a function on the kv so that it can be tested. You should test both the successfull flow (begin tx -> commit) and the exception flow (begin tx -> rollback)
ctx.kv = new KVStore(db, finality, endCursor); | |
await db.exec("BEGIN TRANSACTION"); | |
ctx.kv = new KVStore(db, finality, endCursor); | |
await ctx.kv.beginTransaction(); |
packages/indexer/src/plugins/kv.ts
Outdated
await db.exec("COMMIT TRANSACTION"); | ||
|
||
const ctx = useIndexerContext(); | ||
|
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.
await db.exec("COMMIT TRANSACTION"); | |
const ctx = useIndexerContext(); | |
const ctx = useIndexerContext(); | |
ctx.kv.commitTransaction() |
packages/indexer/src/plugins/kv.ts
Outdated
await db.exec("ROLLBACK TRANSACTION"); | ||
|
||
const ctx = useIndexerContext(); |
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.
await db.exec("ROLLBACK TRANSACTION"); | |
const ctx = useIndexerContext(); | |
const ctx = useIndexerContext(); | |
ctx.kv.rollbackTransaction(); |
add
kv
plugin,useKVStore
hook andKVStore
test. also updatebiome
config to check for unused imports