rttanalysis: add benchmark for hasura's introspection query#90780
rttanalysis: add benchmark for hasura's introspection query#90780craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
0f68bfc to
3ea70d7
Compare
DrewKimball
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @ajwerner and @ZhouXing19)
|
|
||
| LEFT JOIN LATERAL | ||
| ( SELECT | ||
| pg_catalog.col_description("table".oid, "column".attnum) as description |
There was a problem hiding this comment.
I think the fix here is to vectorize this builtin so we can do the fetching in parallel. At least, I think that's the simplest approach. @chengxiong-ruan is working on cleaning up some of the comment access but I don't think it's going to be easy to leverage that to pre-fetch the right comments.
There was a problem hiding this comment.
I think it'd be nice to have some more tests with more tables and more columns so that the growth in terms of round-trips is clear.
There was a problem hiding this comment.
I don't think there is any growth in round-trips based on number of tables
There was a problem hiding this comment.
Can you prove me wrong? I expect for every column in every table that we'll invoke pg_catalog.col_description which will query system.comments, no?
There was a problem hiding this comment.
oh to be clear, i made that comment after modifying the benchmark setup and trying it, and observing no change. i can commit that though if it's worthwhile
There was a problem hiding this comment.
Hmm I wonder if we have a bug in rttanalysis!
There was a problem hiding this comment.
I feel like the apply join trace doesn't propagate correctly.
There was a problem hiding this comment.
I tried it out and it seems like what's going on is that the trace is so big that it's being truncated 😓
There was a problem hiding this comment.
ah darn... well then i will go ahead and merge this for now, and if are able to workaround that truncation in the future we can update the benchmark at that time.
This query exposes an issue with how we construct filters for certain types of joins. It can be used to verify future enhancements. Release note: None
3ea70d7 to
2193506
Compare
|
bors r+ |
|
Build succeeded: |
|
This test with 8 tables seems to take around 6m in CI. I'm inclined to suggest that we skip it until we optimize it. |
|
|
informs #88885
This query exposes an issue with how we construct filters for certain types of joins. It can be used to verify future enhancements.
Release note: None