Skip to content
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

lifinity remove pools CTE #6344

Merged
merged 7 commits into from
Aug 1, 2024
Merged

lifinity remove pools CTE #6344

merged 7 commits into from
Aug 1, 2024

Conversation

andrewhong5297
Copy link
Collaborator

pools inner join on token accounts fungible was wrong, pools were actually NFTs. I went ahead and just removed the whole pools CTE since we don't need it anymore as the data is in transfers. this should run much faster now.

@andrewhong5297
Copy link
Collaborator Author

andrewhong5297 commented Jul 9, 2024

once this runs, we should check the CI output against this query to see if there are missing trades. honestly should maybe generally have a test that checks for tx_id from decoded table in final trades table
https://dune.com/queries/3905260

@andrewhong5297 andrewhong5297 added the ready-for-review this PR development is complete, please review label Jul 11, 2024
@jeff-dude
Copy link
Member

is the issue here just being blocked by CI running? if so, we can look to increase the size and try again.

once this runs, we should check the CI output against this query to see if there are missing trades. honestly should maybe generally have a test that checks for tx_id from decoded table in final trades table https://dune.com/queries/3905260

this may be a bit difficult, as we rename CI tables to be unique and you'd want to lookup to the CI table you just built for the test. for now, it may be best to just do this type of testing during the development phase.

@jeff-dude jeff-dude self-assigned this Jul 11, 2024
@jeff-dude jeff-dude added question Further information is requested in review Assignee is currently reviewing the PR dbt: solana covers the Solana dbt subproject and removed ready-for-review this PR development is complete, please review labels Jul 11, 2024
@andrewhong5297
Copy link
Collaborator Author

Yeah CI is timing out but this should otherwise be fine.

@jeff-dude
Copy link
Member

Yeah CI is timing out but this should otherwise be fine.

hmm slightly concerning that it still times out on a cluster more than double the size. i will have to try again early next week and monitor closely. also will be curious to see history of these two full refreshes on old code, which in theory should be slower since it had more joins and such.

@jeff-dude jeff-dude removed the question Further information is requested label Jul 12, 2024
@andrewhong5297
Copy link
Collaborator Author

Wait we never merged this!! This is a big bug we need this merged

@jeff-dude
Copy link
Member

Wait we never merged this!! This is a big bug we need this merged

this one was still timing out on CI cluster after i made it the size of prod, which isn't a great sign. i can give it another go today and hopefully see it run.

@jeff-dude
Copy link
Member

it worked! phew. i feel more confident pushing this through now.

@jeff-dude jeff-dude added ready-for-merging and removed in review Assignee is currently reviewing the PR labels Aug 1, 2024
@andrewhong5297
Copy link
Collaborator Author

Haha awesome thats a relief! Thanks for trying it

@jeff-dude jeff-dude merged commit 6f8d5e7 into main Aug 1, 2024
2 checks passed
@jeff-dude jeff-dude deleted the lifinity-fix branch August 1, 2024 17:21
@github-actions github-actions bot locked and limited conversation to collaborators Aug 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dbt: solana covers the Solana dbt subproject ready-for-merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants