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

Harden parallel execution #869

Merged
merged 10 commits into from Jul 12, 2017

Conversation

Projects
None yet
4 participants
@codito
Contributor

codito commented Jun 15, 2017

  • Update termination condition to use sources count
  • Handle a scenario where the concurrent execution manager thread can save leading
    to parallel execution hang
  • Send abort in case of exception during Discovery or Execution in the runner. This will ensure a new execution manager is spawned to replace in case of abort.
  • Do not count tests in case a discovery aborts for a parallel discovery scenario.

codito added some commits Jun 15, 2017

Harden parallel execution.
* Update termination condition to use sources count
* Handle a scenario where the concurrent execution manager thread can save leading
to parallel execution hang
Send abort in case of exception during Discovery or Execution in
the runner. This will ensure a new execution manager is spawned to replace
in case of abort.

Do not count tests in case a discovery aborts for a parallel discovery scenario.

@codito codito requested review from smadala and Faizan2304 Jun 15, 2017

}
else
{
// Set the enumerator for parallel yielding of sources
// Whenever a concurrent executor becomes free, it picks up the next source using this enumerator
this.sourceEnumerator = testRunCriteria.Sources.GetEnumerator();
this.availableTestSources = testRunCriteria.Sources.Count();

This comment has been minimized.

@singhsarab

singhsarab Jul 5, 2017

Member

availableTestSources [](start = 21, length = 20)

nit: should we rename it to TestSourceCount ?

@singhsarab

singhsarab Jul 5, 2017

Member

availableTestSources [](start = 21, length = 20)

nit: should we rename it to TestSourceCount ?

This comment has been minimized.

@Faizan2304

Faizan2304 Jul 6, 2017

Contributor

I am fine with name but if you are changing here then please change in ParallelProxyDiscoveryManager.cs also

@Faizan2304

Faizan2304 Jul 6, 2017

Contributor

I am fine with name but if you are changing here then please change in ParallelProxyDiscoveryManager.cs also

@singhsarab

This comment has been minimized.

Show comment
Hide comment
@singhsarab
Member

singhsarab commented Jul 5, 2017

:shipit:

@@ -22,6 +24,8 @@ internal class ParallelProxyExecutionManager : ParallelOperationManager<IProxyEx
#region TestRunSpecificData
private int runCompletedClients = 0;
private int runStartedClients = 0;

This comment has been minimized.

@Faizan2304

Faizan2304 Jul 6, 2017

Contributor

Can you please explain why we need this variable? Why runCompletedClients is not fulfilling our requirement?

@Faizan2304

Faizan2304 Jul 6, 2017

Contributor

Can you please explain why we need this variable? Why runCompletedClients is not fulfilling our requirement?

@Faizan2304

I have a very small comment, rest look very fine to me 👍 :shipit:

@codito codito merged commit cbd68d3 into Microsoft:master Jul 12, 2017

4 checks passed

Ubuntu16.04 / Debug Build Build finished.
Details
Ubuntu16.04 / Release Build Build finished.
Details
Windows_NT / Debug Build Build finished.
Details
Windows_NT / Release Build Build finished.
Details

@codito codito deleted the codito:parallel-exec branch Jul 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment