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

Add Solana gas fees model #6926

Merged
merged 36 commits into from
Oct 18, 2024
Merged

Add Solana gas fees model #6926

merged 36 commits into from
Oct 18, 2024

Conversation

0xBoxer
Copy link
Collaborator

@0xBoxer 0xBoxer commented Oct 10, 2024

This PR adds the Solana gas fees model, excluding source and documentation parts.

Copy link

@Deluaney1 Deluaney1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent

@0xRobin 0xRobin added WIP work in progress blocked and removed blocked labels Oct 14, 2024
@0xBoxer
Copy link
Collaborator Author

0xBoxer commented Oct 14, 2024

logic checks pass and I think I caught all edgecases now.
for some reason the incremental build is slower than the non-incremental build even though it should be 1d versus 9 days, can't really see why, need help debugging.

@0xBoxer
Copy link
Collaborator Author

0xBoxer commented Oct 14, 2024

the incremental build spits out errors because it doesn't have prices for the most recent transactions, could hard limit to some 30 minute delay or smth cut-off date

@0xBoxer
Copy link
Collaborator Author

0xBoxer commented Oct 14, 2024

will open up non-incremental to a month now to see how that changes runtimes

@0xRobin
Copy link
Collaborator

0xRobin commented Oct 15, 2024

  • runtime optimization - I think I did all I can here to optimize the joins and operations tbh, but would appreciate a second set of eyes

looks good to me.

  • prices.usd lags behind by a few minutes, so the prices left join for the most recent transactions fails, do we solve that by using prices.usd_latest usually?

we can use prices.usd_forward_fill which is a view that does a forward fill for the last hour to catch any price lags

Copy link
Collaborator

@0xRobin 0xRobin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good to me, I don't see any further reasons why incremental is taking longer.

@0xBoxer
Copy link
Collaborator Author

0xBoxer commented Oct 16, 2024

also don't have anything more to change here, should be good to merge

@0xBoxer 0xBoxer added ready-for-final-review and removed WIP work in progress labels Oct 16, 2024
@0xBoxer
Copy link
Collaborator Author

0xBoxer commented Oct 16, 2024

before merging we need to take out all {{if not is incremental}} filters

@jeff-dude
Copy link
Member

this looks good to me, I don't see any further reasons why incremental is taking longer.

agreed on setup looking good. can't help but wonder how much incremental time increases when history is expanded extensively (larger table to lookup on in merge). this is only on 10 days of history? oof.

will require us to watch first run after merge + next few incremental to see how it's looking for sure

@jeff-dude jeff-dude added the dbt: solana covers the Solana dbt subproject label Oct 16, 2024
@0xBoxer
Copy link
Collaborator Author

0xBoxer commented Oct 16, 2024

This incremental predicate should keep it from getting slower though. The amount of hashes to merge into should stay roughly static if this limits the range of hashes to compare against.

incremental_predicates = [incremental_predicate('DBT_INTERNAL_DEST.block_time')],

@jeff-dude
Copy link
Member

This incremental predicate should keep it from getting slower though. The amount of hashes to merge into should stay roughly static if this limits the range of hashes to compare against.

incremental_predicates = [incremental_predicate('DBT_INTERNAL_DEST.block_time')],

oh yes, of course, should be good

@jeff-dude
Copy link
Member

i wonder if our issue on incremental could be around block_date and block_time being different columns in solana raw data, and the engine can't tie them together in terms of filter --> partition?

on evm raw tables, they are views where block_date is built off block_time raw data. the engine can tie them together from that and infer when searching partitions.

here we set block_date as partition, but filter incrementally on block_time and maybe the engine isn't catching that relationship?

@0xBoxer
Copy link
Collaborator Author

0xBoxer commented Oct 16, 2024

reasonable hypothesis - can we change { incremental_predicate('block_time') }} to {{incremental_predicate('block_date') }} ?

no way to loose the connection that way

@jeff-dude
Copy link
Member

doesn't hurt to give it a shot. we could also update the source incremental filters to match the updated target one

@0xRobin
Copy link
Collaborator

0xRobin commented Oct 17, 2024

I'll run some tests to see

@0xRobin
Copy link
Collaborator

0xRobin commented Oct 17, 2024

ok, this is as optimized as we can get it. I guess it's just the raw amount of rows that is causing the incremental to take so long :(
I'll merge this tomorrow morning.

@0xRobin 0xRobin merged commit 8e6d2b4 into main Oct 18, 2024
2 checks passed
@0xRobin 0xRobin deleted the add_solana_gas_fees branch October 18, 2024 07:28
@github-actions github-actions bot locked and limited conversation to collaborators Oct 18, 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.

4 participants