Reorganize comment and remove unnessary message to reduce confusion #4407
Conversation
Signed-off-by: Sophia Guo <[email protected]>
Co-authored-by: Shelley Lambert <[email protected]>
| @@ -230,8 +226,11 @@ timestamps{ | |||
| } | |||
| } | |||
| } | |||
| // No agents available, start VM agent. | |||
| // When Parallel the race condition could happen. Say the number of multiply jobs is larger than the available nodes the query's result may be delayed and wrong | |||
There was a problem hiding this comment.
| // When Parallel the race condition could happen. Say the number of multiply jobs is larger than the available nodes the query's result may be delayed and wrong | |
| // When running in parallel, a race condition could occur. If for example, the number of multiply jobs is greater than the available nodes, then the query's result may be delayed and incorrect |
| @@ -230,8 +226,11 @@ timestamps{ | |||
| } | |||
| } | |||
| } | |||
| // No agents available, start VM agent. | |||
| // When Parallel the race condition could happen. Say the number of multiply jobs is larger than the available nodes the query's result may be delayed and wrong | |||
| // In this case jobs will be fooled to fall back to wait local busy nodes. | |||
There was a problem hiding this comment.
I don't quite understand this comment.
There was a problem hiding this comment.
We have too many different terms in the comments for just 2 main things that we are working with in this piece of code:
static Jenkins nodesdynamic agents
We could stick to those terms, and replace anything else to avoid confusion. Wherevirtual agent,VM agent,vmterms are used, we could replace with one term,dynamic agent. Whereidle nodes,agents,nodesterms are used, could replace with one term,static Jenkins nodes.
The idea of this code is:
"If dynamic agents are supported for this platform, then we consider to use them, but we prefer to utilize the static Jenkins nodes first, if they are available. If no static Jenkins nodes are available, spin up a dynamic agent.
If a bunch of jobs get launched at once, because a parent job triggered parallel jobs to be run, there may be a situation, where all of those jobs ask the same question at the same time, are there static Jenkins nodes available? The response to all of those simultaneous questions may be delayed and no longer valid. In this situation, this code will fall back to wait for static Jenkins nodes to become available, instead of spinning up a dynamic agent."
Not suitable for code comments, but for separate documentation that we should also write, it may be easier to understand with an example:
3 static Jenkins nodes are available, a parent job requests to run tests in parallel across 4 nodes. All 4 child jobs run this setup code and ask if static nodes are available. Because they all ask at the same time, the Jenkins server may respond to all 4 child jobs that static nodes are available, but only 3 will get deployed onto those nodes. The 4th child job, will now wait for a static Jenkins node to become available, rather than spinning up a dynamic agent.
|
@sophia-guo - do you still want to try to update this one to get it in? |
|
Needs a rebase |
|
Close this one as outdated. |
Close #4404
Signed-off-by: Sophia Guo [email protected]