router icon indicating copy to clipboard operation
router copied to clipboard

Add QueryPlannerConfig to QueryPlan key

Open fotoetienne opened this issue 1 year ago • 4 comments
trafficstars

Addresses Issue #5093

This is an incomplete PR (doesn't build yet) that demonstrates a proposed solution to Issue #5093

fotoetienne avatar May 06 '24 22:05 fotoetienne

@fotoetienne: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

apollo-cla avatar May 06 '24 22:05 apollo-cla

CI performance tests

  • [x] step - Basic stress test that steps up the number of users over time
  • [ ] events_big_cap_high_rate_callback - Stress test for events with a lot of users, deduplication enabled and high rate event with a big queue capacity using callback mode
  • [ ] large-request - Stress test with a 1 MB request payload
  • [ ] events - Stress test for events with a lot of users and deduplication ENABLED
  • [ ] xxlarge-request - Stress test with 100 MB request payload
  • [ ] events_without_dedup - Stress test for events with a lot of users and deduplication DISABLED
  • [ ] xlarge-request - Stress test with 10 MB request payload
  • [ ] step-jemalloc-tuning - Clone of the basic stress test for jemalloc tuning
  • [ ] events_callback - Stress test for events with a lot of users and deduplication ENABLED in callback mode
  • [ ] no-graphos - Basic stress test, no GraphOS.
  • [ ] reload - Reload test over a long period of time at a constant rate of users
  • [ ] events_big_cap_high_rate - Stress test for events with a lot of users, deduplication enabled and high rate event with a big queue capacity
  • [ ] events_without_dedup_callback - Stress test for events with a lot of users and deduplication DISABLED using callback mode
  • [x] const - Basic stress test that runs with a constant number of users

router-perf[bot] avatar May 06 '24 22:05 router-perf[bot]

this sounds ok, the only thing missing right now is hashing the planner config along with the metadata and others at https://github.com/apollographql/router/pull/5094/files#diff-560bfe015eb92a1ce9b2b1e2282f58d41574eeb3cb1d94385b5c8b96544e8b1cR546 (This part generates the redis cache key)

Then probably some tests will need to be updated, in particular the redis integration test. Do you want me to get it to the finish line or do you want to attempt it?

If you have the bandwidth, that would be great. Thanks!

fotoetienne avatar May 07 '24 20:05 fotoetienne

We'll take a look!

abernix avatar May 08 '24 11:05 abernix

I'll close this in lieu of https://github.com/apollographql/router/pull/5100 which took a similar approach to solve for the intent of this PR, along with taking some other considerations. Appreciate you taking the time to do this follow-up and prompting the work, nonetheless. 😄

abernix avatar Jun 10 '24 07:06 abernix