[SERVER-71165] Consider adding aggregation/ tests to applicable passthroughs Created: 08/Nov/22  Updated: 08/Aug/23

Status: Backlog
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major - P3
Reporter: Judah Schvimer Assignee: Backlog - Query Execution
Resolution: Unresolved Votes: 0
Labels: greenerbuild
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to SERVER-60886 Add an aggregation multiversion suite Backlog
is related to SERVER-77844 Add aggregation/ tests to NTDI passth... Closed
Assigned Teams:
Query Execution
Participants:

 Description   

Aggregations increasingly do writes so they are worth putting through our passthroughs as applicable. Currently they're not included in jscore passthroughs and have some of their own. We should consider if there are passthroughs (like the initial sync ones) that are worth including them in.



 Comments   
Comment by Janna Golden [ 27/Jul/23 ]

sebastien.mendez@mongodb.com we ended up filing SERVER-77844 to specifically add the aggregation tests to some of our own passthroughs, but not into passthroughs that other teams own. I think it may make sense to leave this ticket on Query's backlog to consider a more widespread change (e.g. moving aggregation/ into core/ or adding these tests into a larger set of passthrough suites).

Comment by Sebastien Mendez [ 27/Jul/23 ]

janna.golden@mongodb.com - Is it okay to re-assign it to backlog-server-serverless for further triage?

And of course, if needed, we'll find a Query reviewer.

Comment by Janna Golden [ 31/May/23 ]

kyle.suarez@mongodb.com - got it, I think we'll likely add either a new passthrough suite, or add the the agg directory to some of our passthrough suites then - happy to add a query reviewer. Thanks for getting back to me!

Comment by Kyle Suarez [ 31/May/23 ]

janna.golden@mongodb.com, we haven't had time to get to this in the Query Execution triage queue, but almost certainly I don't think we have the bandwidth to schedule this soon. I wouldn't be opposed to your team adding a new passthrough suite if someone on Query reviewed it and signed off.

CC judah.schvimer@mongodb.com

Comment by Janna Golden [ 15/May/23 ]

Hey all (will.buerger@mongodb.com, kyle.suarez@mongodb.com, etc.)! Is there any update on whether we'll be doing this? We had opted to hold off on standing up a new aggregation passthrough suite in PM-2346 because of this work, but if we are not planning on doing this (or, not in the next month), then we would likely want to add that suite.

cc sophia.tan@mongodb.com

Comment by Will Buerger [ 27/Apr/23 ]

TIL I have to mark this for it to be brought up in triage again!

Comment by Will Buerger [ 21/Mar/23 ]

Hey folks! Sorry for the delay - I was out for consecutive BF days, but finally got around to investigating this last week.

The main takeaways from trying to move over `jstests/aggregation` into `jstests/core/aggregation` are:

(1) It does cause a large number of individual test errors, but — as far as I could tell without really diving into each one — many if not most of the errors are resolved by adding test tags, as the test was just run in some passthrough that it isn’t compatible with.

(2) Evergreen task generation fails when the testing diff is too large, so it gets overwhelmed when moving over more than a handful of tests (unclear exactly where the threshold is, but somewhere between 10-40 files at a time — jstests/aggregation has 385 files). So if we decided to do it in one massive patch, we’d lose coverage on burn-in tests and the other generated tests. From a brief investigation, it seems no one’s found a way around this issue other than splitting the patch to a size acceptable by Evergreen.

If we do decide this change is worth it, one possibility would be to split the move up into chunks digestible by Evergreen (if it accepts 20 files moved at a time, that’d be about 20 separate patches). With that approach, we can resolve the test tag issues incrementally and catch any other errors along the way. It’d take some time but would be a reasonably safe approach.

Comment by Dan Larkin-York [ 20/Mar/23 ]

I'm 100% on-board with the idea of getting better aggregation test coverage, and this seems like a straightforward way to do so, but I'd like us to consider the costs, since it's not really "free".

Do we know anything about the run time of these tests compared to that of the current core suite? By pure test count, this would result in a ~30% increase in the core suite. Given the number of passthroughs, this could represent a significant increase in machine time (monetary cost) and potentially wall-clock time if not parallelized appropriately (developer productivity cost from slower patch builds).

This may very well be worth it, given the scope of added coverage, but I think we should consider it carefully. Are there any more targeted approaches to consider?

Comment by Kyle Suarez [ 16/Feb/23 ]

will.buerger@mongodb.com has graciously volunteered to try this during BF days; if it turns out to cause lots of failures we will regroup and reconsider how much effort this might be.

Comment by Kyle Suarez [ 10/Feb/23 ]

After consulting with bernard.gorman@mongodb.com, we've decided that Arun's approach sounds very reasonable and this is worth an attempt during one of the BF Thursday/Fridays. Removing the director triage label and tagging this with greenerbuild.

Comment by Arun Banala [ 02/Feb/23 ]

Now that the jstests/core tests are reorganized to have sub-directories, we should consider moving jstests/aggregation to jstests/core/aggregation. If we do this, we will get a lot of core passthrough coverage for free (and we can avoid doing tickets like SERVER-60886). Most of the aggregation passthroughs (like aggregation_ese, aggregation_secondary_reads, are clones of similar core passthroughs. We can also eliminate all these duplicates with this approach.

Comment by Judah Schvimer [ 15/Nov/22 ]

This was not motivated by a bug, but inspired by SERVER-71139 (which this would not have caught, but made me think about ways we can increase testing gaps like SERVER-71166).

Comment by Steve Tarzia [ 15/Nov/22 ]

judah.schvimer@mongodb.com is this motivated by a specific bug or is this just a general cleanup idea?

Generated at Thu Feb 08 06:18:14 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.