search

pact-foundation / pact_broker

Enables your consumer driven contracts workflow
http://pactflow.io
MIT License
708 stars 174 forks source link

Aggregate pacts with duplicate interactions into a single "uber pact" to reduce verification time #710

Open ertrzyiks opened 3 years ago

ertrzyiks commented 3 years ago

Let's say our contract file has 100 interactions.

When we create a feature branch adding 1 more interactions we have 2 contracts: A. 100 interactions B. 100 interactions from contract A + 1 new interaction

The provider verification picks both contract files (either specifically, either as WIP). Currently, all 201 interactions are checked even 100 of them are verified twice. Could we reuse a single interaction verification results to optimize it, so only 101 interactions are actually performed?

For the background, when we have multiple WIP branches our job execution time jumped from 9 minutes to 50 minutes while most of the work is doing the same thing over and over.

mefellows commented 3 years ago

I think this has come up before, at the very least, I've raised this idea before (not sure... where, probably on pact-broker). Beth did some investigation a while back using data from our Pactflow customer base. We resolved that it wasn't a big saving.

That may have been before WIP/Pending though, which I can see would change this situation.

In any case, keen to see if others have this problem too.

(Also, probably should be moved to pact-broker if it's not a dupe)

bethesque commented 3 years ago

I thought I'd already created a feature request for this somewhere, but I can't find it. I've created it here: https://pact.canny.io/feature-requests/p/aggregate-pacts-with-duplicate-interactions-into-a-single-uber-pact-to-reduce-ve

I did some numbers on this, and determined that for a small number of users, it would make a big difference, but for the majority, it wouldn't make much difference. You're obviously in the small number of users for whom it would make a big difference @ertrzyiks. 50 minutes is just ridiculous. Is this why you're keen to get that wip selectors thing? What's the simplest thing I can do to make that quicker?

bethesque commented 3 years ago

@ertrzyiks are you across this? https://github.com/pactflow/roadmap/issues/4 If your builds are taking that long, they might be a good candidate for bi-directional contracts.

ertrzyiks commented 3 years ago

Thanks for the fast reaction @mefellows @bethesque !

@bethesque I was not aware of bi-directional contract, I need to read about it, thanks for the hint.

Is this why you're keen to get that wip selectors thing?

It's part of the problem, yeah. We wanted to evaluate if enabling WIP verification is feasible in our system, so the initial plan was to introduce it just for one pair Provider-Consumer. Another thing is that some of our Provider-Consumer pairs are just being set up so the broker is filled with more data than we care about at that point. WIP selector picks contracts that we then never check with can-i-deploy and it is waste of resources.

What's the simplest thing I can do to make that quicker

I created this issue in the ruby gem because the system in question uses ruby. If you are unsure if this feature should make it to the pact spec for all the languages, maybe we can have an option to enable optimization like enable_experimental_uber_pact and skip interactions that were already checked? If you think it makes sense I can try to submit a PR, at least with some draft idea.

For now, we disabled WIP verification so the job is fast again. It reintroduced the issues we aimed to fix with WIP, so eventually it would be great to bring WIP verification back but the performance issue is a real pain to us.

ertrzyiks commented 3 years ago

@bethesque any thoughts on how / if we want to push this idea forward? I briefly checked the bi-directional contracts, but I see it more suitable for REST API (please correct me if I'm wrong) while our contracts cover graphql queries and mutations.

mefellows commented 3 years ago

Yes, currently bi-directional contracts only supports an OAS backed provider - GraphQL is in the plans, but for now we'll need to find another way forward.

bethesque commented 3 years ago

I've done a proof of concept to aggregate all the interactions into a single uber-pact on the broker side. It needs a bit more work - the interactions have to be grouped by provider/consumer/WIP status/pact specification version, and then the results have to be spread back out into their respective source pacts, but I believe it's possible. Unfortunately, I don't know when I'm going to have time to do it, because I need to finish the branches/env support work before I pick up anything else. If you're interested, I can help you or someone with the work.