Store validated ledgers inside a persistent DB #370
Conversation
…penHandles/memory-leak error
ckeshava
changed the title
| | `connected` |Boolean denoting websocket connection status. | | ||
| | `status_update_time` |Time when the connected column was updated. | | ||
|
|
||
| ### `validated_ledgers` |
There was a problem hiding this comment.
Lets update this after all comments are addressed.
| * @param networkName - The name of the network. | ||
| * @returns A promise that resolves when the insertion is complete. | ||
| */ | ||
| export async function insertValidations( |
There was a problem hiding this comment.
-
If there is exactly one entry we update it. If there are 0 entries we log a message that you are already logging and insert the row as well (to account for validation that we receive for which we never had a ledger message). Last
elseclause should never occur but good to have it for logging purposes. -
try catch is required here, otherwise DB operation failure will fail the actual source code flow.
There was a problem hiding this comment.
I will review updated test cases once, we conclude on these changes and other changes.
There was a problem hiding this comment.
If there are 0 entries we log a message that you are already logging and insert the row as well (to account for validation that we receive for which we never had a ledger message).
Regarding this case, if the VHS has not recieved the ledgerClosed message for a specific ledger, then I cannot insert the row. I do not have the necessary fields of the StreamLedger interface to be able to fill out these values at this stage. Executing a log.error() appears to be the best course of action here.
There was a problem hiding this comment.
addressed point-2 in the commit: 84740c0
| ledger_hashes.push(current_ledger.ledger_hash) | ||
|
|
||
| if (networks) { | ||
| await insertValidatedLedger(networks, current_ledger) |
There was a problem hiding this comment.
I think we are not de-duplicating when hitting database. Not sure if this will lead to deadlock.
try catch in insertValidatedLedger would prevent node.js terminating, but not the deadlock if it ever occurs.
If we want to keep this as it is, I would suggest deploying this branch on staging/dev and then deploying it in production if it works well.
There was a problem hiding this comment.
On line 33, I'm checking against for duplicates against the primary-key of the db. Given that these 3 columns make the primary-key, I expect this operation to be optimized by the DB.
Besides, I'm not executing any other instruction in that method. Hence, there is no possibility of ungraceful-closure of the db operation (or) leakage of DB-connections in this method.
Sure, we can observe in dev/stg environment for a few days.
There was a problem hiding this comment.
Not blocking the PR review: I would recommend testing it in staging/dev for a few days.
|
@ckeshava This branch |
Update: I have deployed this branch on |
3 participants
High Level Overview of Change
This PR aims to comprehensively track all the validated and missing ledgers, as recorded by the VHS. It also exposes suitable API endpoints to query this piece of information.
If you prefer the API endpoints (or) the DB schema differently for your use-cases, let me know.
The
validated_ledgerstable is pruned daily. I have not set up a pruning schedule formissing_ledgerstable yet for debugging purposes. Presently, both these tables only track the mainnet for reducing the storage burden. If deemed necessary, this can be extended to other networks as well.Context of Change
Type of Change
Before / After
Here is an example of how I envision the usage of this PR changes:
Debugging: Inspect the
missing-ledgerstable. Note theledger_index,previous_ledgerandprevious_received_attimestamps.Use the information from the contextual rows in the
validated_ledgerstable to gather more information about the timestamp of the missing ledger. Use this timestamp to correlate events with the logs ofvhs-connectionspod. (Did the pod restart? Was there a prior unhandled error? etc)Source of Truth: The
missing_ledgersandvalidated_ledgersprovide a comprehensive store of the VHS's data. It can be used to pinpoint erroneous behavior.If one does not have access to the DB, this can be accomplished with the exposed API endpoints as well.
Test Plan
Unit tests have been added for the DB tasks pertaining to
validated_ledgerstable. If we are planning to retain themissing_ledgerstable in the production VHS instance, suitable tests can be added in the future.Future Tasks
In a future PR, I'd like to track all the incoming validations received by the VHS.