Skip to content

Commit 308bce3

Browse files
larsrc-googlecopybara-github
authored andcommitted
Actively kill off still-active workers when stopping work on interrupt.
Due to limits of GenericKeyedObjectPool, we can't selectively kill the active workers, but must instead clear the entire pool. RELNOTES: None. PiperOrigin-RevId: 352441326
1 parent fa9fabb commit 308bce3

File tree

3 files changed

+78
-4
lines changed

3 files changed

+78
-4
lines changed

src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,12 +203,20 @@ public void buildComplete(BuildCompleteEvent event) {
203203
}
204204
}
205205

206-
// Kill workers on Ctrl-C to quickly end the interrupted build.
207-
// TODO(philwo) - make sure that this actually *kills* the workers and not just politely waits
208-
// for them to finish.
206+
/**
207+
* Stops any workers that are still executing.
208+
*
209+
* <p>This currently kills off some amount of workers, losing the warmed-up state.
210+
* TODO(b/119701157): Cancel running workers instead (requires some way to reach each worker).
211+
*/
209212
@Subscribe
210213
public void buildInterrupted(BuildInterruptedEvent event) {
211-
shutdownPool("Build interrupted, shutting down worker pool...");
214+
if (workerPool != null) {
215+
if ((options != null && options.workerVerbose)) {
216+
env.getReporter().handle(Event.info("Build interrupted, stopping active workers..."));
217+
}
218+
workerPool.stopWork();
219+
}
212220
}
213221

214222
/** Shuts down the worker pool and sets {#code workerPool} to null. */

src/main/java/com/google/devtools/build/lib/worker/WorkerPool.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,4 +193,24 @@ public void close() {
193193
workerPools.values().forEach(GenericKeyedObjectPool::close);
194194
multiplexPools.values().forEach(GenericKeyedObjectPool::close);
195195
}
196+
197+
/** Stops any ongoing work in the worker pools. This may entail killing the worker processes. */
198+
public void stopWork() {
199+
workerPools
200+
.values()
201+
.forEach(
202+
pool -> {
203+
if (pool.getNumActive() > 0) {
204+
pool.clear();
205+
}
206+
});
207+
multiplexPools
208+
.values()
209+
.forEach(
210+
pool -> {
211+
if (pool.getNumActive() > 0) {
212+
pool.clear();
213+
}
214+
});
215+
}
196216
}

src/test/java/com/google/devtools/build/lib/worker/WorkerPoolTest.java

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,4 +238,50 @@ public void testBorrow_twoHiPrioBlocks() throws Exception {
238238
verify(factoryMock, times(2)).makeObject(workerKey1);
239239
verify(factoryMock, times(1)).makeObject(workerKey2);
240240
}
241+
242+
@Test
243+
public void testStopWork_activePoolsStopped() throws Exception {
244+
WorkerPool pool =
245+
new WorkerPool(
246+
factoryMock,
247+
// Have to declare the mnemonics, or they all fall into the default SimpleWorkerPool.
248+
ImmutableMap.of("mnem1", 2, "mnem2", 2),
249+
ImmutableMap.of("mnem2", 2, "mnem3", 2),
250+
Lists.newArrayList());
251+
WorkerKey singleKey1 = createWorkerKey(fileSystem, "mnem1", false);
252+
// These workers get borrowed, then both get destroyed in stopWork because they share mnemonic
253+
WorkerKey singleKey1a = createWorkerKey(fileSystem, "mnem1", false, "anArg");
254+
pool.borrowObject(singleKey1);
255+
Worker worker1a = pool.borrowObject(singleKey1a);
256+
pool.returnObject(singleKey1a, worker1a);
257+
WorkerKey singleKey2 = createWorkerKey(fileSystem, "mnem2", false);
258+
// This worker gets borrowed, then returned, doesn't get destroyed in stopWork
259+
Worker worker1 = pool.borrowObject(singleKey2);
260+
pool.returnObject(singleKey2, worker1);
261+
WorkerKey multiKey1 = createWorkerKey(fileSystem, "mnem2", true);
262+
// This worker gets borrowed, then destroyed in stopWork, but separately from the singleplex
263+
// worker2 even though they share a mnemonic.
264+
pool.borrowObject(multiKey1);
265+
WorkerKey multiKey2 = createWorkerKey(fileSystem, "mnem3", true);
266+
// This worker gets borrowed, then returned, doesn't get destroyed during stopWork.
267+
Worker worker2 = pool.borrowObject(multiKey2);
268+
pool.returnObject(multiKey2, worker2);
269+
verify(factoryMock, times(1)).makeObject(singleKey1);
270+
verify(factoryMock, times(1)).makeObject(singleKey1a);
271+
verify(factoryMock, times(1)).makeObject(singleKey2);
272+
verify(factoryMock, times(1)).makeObject(multiKey1);
273+
verify(factoryMock, times(1)).makeObject(multiKey2);
274+
pool.stopWork();
275+
pool.borrowObject(singleKey1);
276+
pool.borrowObject(singleKey1a);
277+
pool.borrowObject(singleKey2);
278+
pool.borrowObject(multiKey1);
279+
pool.borrowObject(multiKey2);
280+
// After stopWork, we had to create new workers for the keys that got their pools destroyed.
281+
verify(factoryMock, times(2)).makeObject(singleKey1);
282+
verify(factoryMock, times(2)).makeObject(singleKey1a);
283+
verify(factoryMock, times(1)).makeObject(singleKey2);
284+
verify(factoryMock, times(2)).makeObject(multiKey1);
285+
verify(factoryMock, times(1)).makeObject(multiKey2);
286+
}
241287
}

0 commit comments

Comments
 (0)