fix: move nft custody view into a table #1741

rafaelcr · 2023-10-19T22:14:18Z

This PR transforms the nft_custody and nft_custody_unanchored materialized views into tables that are written to with each new block/microblock. This migration increases block ingestion performance considerably.

vercel · 2023-10-19T22:14:23Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
stacks-blockchain-api	❌ Failed (Inspect)			Oct 25, 2023 2:41pm

github-actions · 2023-10-19T22:17:01Z

🚀 Deployed on https://deploy-preview-1741--stacks-blockchain-api.netlify.app

codecov · 2023-10-19T22:17:27Z

Codecov Report

Merging #1741 (de27116) into master (cc4d0e5) will decrease coverage by 21.86%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           master    #1741       +/-   ##
===========================================
- Coverage   76.82%   54.96%   -21.86%     
===========================================
  Files          80       80               
  Lines       11515    11510        -5     
  Branches     2575     2575               
===========================================
- Hits         8846     6327     -2519     
- Misses       2540     4815     +2275     
- Partials      129      368      +239

Files	Coverage Δ
src/datastore/common.ts	`100.00% <ø> (ø)`
src/datastore/pg-store.ts	`69.54% <ø> (-21.05%)`	⬇️
src/datastore/pg-write-store.ts	`72.97% <100.00%> (-6.91%)`	⬇️

... and 42 files with indirect coverage changes

zone117x · 2023-10-23T09:41:52Z

Very clean 💯
I'm locally testing out block ingestion times with this

csgui

Good change! 👍 LGTM!

zone117x · 2023-10-25T13:00:40Z

migrations/1696872367486_nft-custody-tables.js

+  pgm.sql(`
+    INSERT INTO nft_custody (asset_identifier, value, recipient, tx_id, block_height) (
+      SELECT asset_identifier, value, recipient, tx_id, block_height
+      FROM nft_custody_old
+    )
+  `);


I'm getting a migration error from this query:

{ "level": "error", "time": "2023-10-25T12:51:37.063Z", "pid": 1, "hostname": "6750bf061f1a", "name": "stacks-blockchain-api", "component": "core-api", "err": { "type": "DatabaseError", "message": "null value in column \"index_block_hash\" of relation \"nft_custody\" violates not-null constraint", "stack": "error: null value in column \"index_block_hash\" of relation \"nft_custody\" violates not-null constraint\n at Parser.parseErrorMessage (/app/node_modules/pg-protocol/src/parser.ts:369:69)\n at Parser.handlePacket (/app/node_modules/pg-protocol/src/parser.ts:188:21)\n at Parser.parse (/app/node_modules/pg-protocol/src/parser.ts:103:30)\n at Socket.<anonymous> (/app/node_modules/pg-protocol/src/index.ts:7:48)\n at Socket.emit (node:events:513:28)\n at addChunk (node:internal/streams/readable:315:12)\n at readableAddChunk (node:internal/streams/readable:289:9)\n at Socket.Readable.push (node:internal/streams/readable:228:10)\n at TCP.onStreamRead (node:internal/stream_base_commons:190:23)", "length": 500, "name": "error", "severity": "ERROR", "code": "23502", "detail": "Failing row contains (SP000000000000000000002Q6VF78.bns::names, \\x0c00000002046e616d6502000000012d096e616d6573706163650200000002..., SPJ5BG9TBW0ZE36ZKK8W59PS1JZT2HSYESR90BKD, 94746, null, null, null, null, \\x445b1dde3854841d4c13a621bfd759ad5fa43ec99c178ebc3937e2d234f205..., null, null).", "schema": "stacks_blockchain_api", "table": "nft_custody", "column": "index_block_hash", "file": "execMain.c", "line": "1883", "routine": "ExecConstraints" }, "msg": "Error running pg-migrate" }

My local deployment is based off a Hiro Archive pg dump (stacks-blockchain-api-pg-15-7.3.0-20230926.dump). If index_block_hash should never be null, then there may be an issue with the archives (perhaps bns import wasn't ran?). Otherwise, could be an issue with this migration query?

Ah I see the issue. The original nft_custody materialized view doesn't have the columns required for this INSERT query. In particular: index_block_hash, parent_index_block_hash, microblock_hash, microblock_sequence, tx_index, event_index.

I think this INSERT query would need to join against the nft_events and/or txs table to retrieve these values.

Should be fixed with this commit: 8a69698

@rafaelcr can you check that this looks correct?

(Reposting message from commit)

This looks correct! Though if this is the case we no longer need to rename the old view, etc. because you're recalculating the table from scratch. This migration will also take much more than a few seconds (probably 5-10 minutes).
I'll update the migration file.

Latest commit works. Migration only takes around 30 seconds on my machine, not bad at all.

zone117x

The migration takes about 30 seconds on my machine, which I think might be a few minutes on the production pg server. Seems okay to me.

I also verified that the block ingestion times are consistently 1-3 seconds with this change.

* docs: Update how-to-install-stacks-cli.md (#1727) Cleaned up the voice and made this page more conversational. Of note, similar to my last PR, I'm removing some language around "Stacks 2.0" here alongside this cleanup * docs: Update rosetta-support.md (#1728) there is a missing period * docs: Update overview.md (#1729) Grammar fixes * docs: Update how-to-run-api-node.md (#1730) A number of grammar fixes and tweaks * docs: Update how-to-run-mainnet-node.md (#1731) Grammar fixes! * docs: Update how-to-run-testnet-node.md (#1732) Grammar fixes * fix: move nft custody view into a table (#1741) * fix: move to table * fix: old nft events query * fix: nft-custody-table migration * fix: no longer rename old views, just remove them --------- Co-authored-by: Matt <[email protected]> --------- Co-authored-by: max-crawford <[email protected]> Co-authored-by: Matt <[email protected]>

## [7.3.3](v7.3.2...v7.3.3) (2023-11-13) ### Bug Fixes * move nft custody view into a table ([#1741](#1741)) ([fb0d0ea](fb0d0ea))

blockstack-devops · 2023-11-13T16:59:22Z

🎉 This PR is included in version 7.3.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

rafaelcr added 2 commits October 19, 2023 15:53

fix: move to table

e616269

fix: old nft events query

f036ea5

github-actions bot temporarily deployed to commit October 19, 2023 22:15 Inactive

github-actions bot temporarily deployed to pull request October 19, 2023 22:17 Inactive

rafaelcr requested a review from csgui October 20, 2023 14:29

rafaelcr marked this pull request as ready for review October 20, 2023 14:29

csgui approved these changes Oct 23, 2023

View reviewed changes

zone117x reviewed Oct 25, 2023

View reviewed changes

fix: nft-custody-table migration

8a69698

vercel bot had a problem deploying to Preview October 25, 2023 13:40 Failure

github-actions bot temporarily deployed to commit October 25, 2023 13:41 Inactive

github-actions bot temporarily deployed to pull request October 25, 2023 13:41 Inactive

zone117x approved these changes Oct 25, 2023

View reviewed changes

zone117x linked an issue Oct 25, 2023 that may be closed by this pull request

SPIKE: Investigate block time ingestion #1715

Closed

fix: no longer rename old views, just remove them

de27116

vercel bot had a problem deploying to Preview October 25, 2023 14:41 Failure

github-actions bot temporarily deployed to pull request October 25, 2023 14:42 Inactive

github-actions bot temporarily deployed to commit October 25, 2023 14:42 Inactive

rafaelcr merged commit fb0d0ea into master Oct 25, 2023
33 of 36 checks passed

rafaelcr deleted the fix/nft-custody branch October 25, 2023 16:48

blockstack-devops pushed a commit that referenced this pull request Nov 13, 2023

chore(release): 7.3.3 [skip ci]

536dbd1

## [7.3.3](v7.3.2...v7.3.3) (2023-11-13) ### Bug Fixes * move nft custody view into a table ([#1741](#1741)) ([fb0d0ea](fb0d0ea))

blockstack-devops added the released label Nov 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: move nft custody view into a table #1741

fix: move nft custody view into a table #1741

rafaelcr commented Oct 19, 2023 •

edited

Loading

vercel bot commented Oct 19, 2023 •

edited

Loading

github-actions bot commented Oct 19, 2023 •

edited

Loading

codecov bot commented Oct 19, 2023 •

edited

Loading

zone117x commented Oct 23, 2023

csgui left a comment

zone117x Oct 25, 2023

zone117x Oct 25, 2023

zone117x Oct 25, 2023

rafaelcr Oct 25, 2023

zone117x Oct 25, 2023

zone117x left a comment

blockstack-devops commented Nov 13, 2023

fix: move nft custody view into a table #1741

fix: move nft custody view into a table #1741

Conversation

rafaelcr commented Oct 19, 2023 • edited Loading

vercel bot commented Oct 19, 2023 • edited Loading

github-actions bot commented Oct 19, 2023 • edited Loading

codecov bot commented Oct 19, 2023 • edited Loading

Codecov Report

zone117x commented Oct 23, 2023

csgui left a comment

Choose a reason for hiding this comment

zone117x Oct 25, 2023

Choose a reason for hiding this comment

zone117x Oct 25, 2023

Choose a reason for hiding this comment

zone117x Oct 25, 2023

Choose a reason for hiding this comment

rafaelcr Oct 25, 2023

Choose a reason for hiding this comment

zone117x Oct 25, 2023

Choose a reason for hiding this comment

zone117x left a comment

Choose a reason for hiding this comment

blockstack-devops commented Nov 13, 2023

rafaelcr commented Oct 19, 2023 •

edited

Loading

vercel bot commented Oct 19, 2023 •

edited

Loading

github-actions bot commented Oct 19, 2023 •

edited

Loading

codecov bot commented Oct 19, 2023 •

edited

Loading