stacks-blockchain-api icon indicating copy to clipboard operation
stacks-blockchain-api copied to clipboard

feat: improve pg write queue

Open bestmike007 opened this issue 1 year ago • 6 comments

Description

This PR is to fix the potential issue described in https://medium.com/@alkor_shikyaro/transactions-and-promises-in-node-js-ca5a3aeb6b74

Type of Change

  • [ ] New feature
  • [x] Bug fix
  • [ ] API reference/documentation update
  • [ ] Other

Does this introduce a breaking change?

No.

Are documentation updates required?

No.

Testing information

Current tests covered this.

Checklist

  • [x] Code is commented where needed
  • [x] Unit test coverage for new or modified code paths
  • [x] npm run test passes
  • [ ] Changelog is updated
  • [x] Tag 1 of @rafaelcr or @zone117x for review

bestmike007 avatar Mar 01 '24 17:03 bestmike007

@zone117x

bestmike007 avatar Mar 01 '24 17:03 bestmike007

Codecov Report

Attention: Patch coverage is 90.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 71.87%. Comparing base (a0856f9) to head (fdb956d).

Files Patch % Lines
src/datastore/helpers.ts 90.00% 1 Missing :warning:
Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #1878   +/-   ##
========================================
  Coverage    71.87%   71.87%           
========================================
  Files           92       92           
  Lines        11913    11920    +7     
  Branches      2624     2625    +1     
========================================
+ Hits          8562     8568    +6     
- Misses        3197     3198    +1     
  Partials       154      154           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 01 '24 17:03 codecov[bot]

@bestmike007 I'm curious to now if this fix was submitted because of a problem you're experiencing? We'd be happy to test some edge case if you've found one we haven't covered yet

rafaelcr avatar Mar 01 '24 20:03 rafaelcr

Yes I was trying to resolve a problem, our nodes run into unrecoverable state, not sure if it's related to this PR.

Here's one of the situations I saw, which should not be happening:

telegram-cloud-photo-size-5-6273870458726235191-y

Another one, I couldn't found a screenshot, was reporting something like table tid from new index tuple (13374,63) overlaps with invalid duplicate tuple at offset 4 of block 6211 in inde, and can be fixed by 3 steps: remove the nft_custody_unique constraints, remove duplicates with lower block heights, and then rebuild the unique constraint.

bestmike007 avatar Mar 02 '24 00:03 bestmike007

Hmm I don't believe that error is related to this change. It seems strange, though, because the ON CONFLICT ON CONSTRAINT is there precisely to avoid this error. Can you tell me more about your setup? Is this error occurring in an event replay, or a normal Stacks node sync?

rafaelcr avatar Mar 06 '24 14:03 rafaelcr

Hmm I don't believe that error is related to this change. It seems strange, though, because the ON CONFLICT ON CONSTRAINT is there precisely to avoid this error. Can you tell me more about your setup? Is this error occurring in an event replay, or a normal Stacks node sync?

It it not related, it's another issue I saw when the node keeps crashing because of unhandledRejection when it fails to process a new_block event.

Our nodes are set up using the script in this repo: https://github.com/alexgo-io/stacks-node-mainnet, and nodes restoring from both hiro archive and the cold backup saw this issue.

bestmike007 avatar Mar 06 '24 16:03 bestmike007