Skip to content

Commit a1144ce

Browse files
joerg1985VietND96
authored and
sandeepsuryaprasad
committed
[grid] delay the newsessionqueue response (SeleniumHQ#14764)
* [grid] delay the newsessionqueue response * Update variable naming with prefix `DEFAULT_` Signed-off-by: Viet Nguyen Duc <[email protected]> --------- Signed-off-by: Viet Nguyen Duc <[email protected]> Co-authored-by: Viet Nguyen Duc <[email protected]>
1 parent 4f0d18c commit a1144ce

19 files changed

+99
-7
lines changed

‎java/src/org/openqa/selenium/grid/commands/Hub.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ protected Handlers createHandlers(Config config) {
144144
distributorOptions.getSlotMatcher(),
145145
newSessionRequestOptions.getSessionRequestTimeoutPeriod(),
146146
newSessionRequestOptions.getSessionRequestTimeout(),
147+
newSessionRequestOptions.getMaximumResponseDelay(),
147148
secret,
148149
newSessionRequestOptions.getBatchSize());
149150
handler.addHandler(queue);

‎java/src/org/openqa/selenium/grid/commands/Standalone.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ protected Handlers createHandlers(Config config) {
150150
distributorOptions.getSlotMatcher(),
151151
newSessionRequestOptions.getSessionRequestTimeoutPeriod(),
152152
newSessionRequestOptions.getSessionRequestTimeout(),
153+
newSessionRequestOptions.getMaximumResponseDelay(),
153154
registrationSecret,
154155
newSessionRequestOptions.getBatchSize());
155156
combinedHandler.addHandler(queue);

‎java/src/org/openqa/selenium/grid/sessionqueue/config/NewSessionQueueOptions.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
public class NewSessionQueueOptions {
3636

3737
static final String SESSION_QUEUE_SECTION = "sessionqueue";
38+
static final int DEFAULT_MAXIMUM_RESPONSE_DELAY = 8;
3839
static final int DEFAULT_REQUEST_TIMEOUT = 300;
3940
static final int DEFAULT_REQUEST_TIMEOUT_PERIOD = 10;
4041
static final int DEFAULT_RETRY_INTERVAL = 15;
@@ -89,6 +90,15 @@ public URI getSessionQueueUri() {
8990
}
9091
}
9192

93+
public Duration getMaximumResponseDelay() {
94+
int timeout =
95+
config
96+
.getInt(SESSION_QUEUE_SECTION, "maximum-response-delay")
97+
.orElse(DEFAULT_MAXIMUM_RESPONSE_DELAY);
98+
99+
return Duration.ofSeconds(timeout);
100+
}
101+
92102
public Duration getSessionRequestTimeout() {
93103
// If the user sets 0 or less, we default to 1s.
94104
int timeout =

‎java/src/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueue.java‎

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ public class LocalNewSessionQueue extends NewSessionQueue implements Closeable {
9696
private static final String NAME = "Local New Session Queue";
9797
private final SlotMatcher slotMatcher;
9898
private final Duration requestTimeout;
99+
private final Duration maximumResponseDelay;
99100
private final int batchSize;
100101
private final Map<RequestId, Data> requests;
101102
private final Map<RequestId, TraceContext> contexts;
@@ -115,6 +116,7 @@ public LocalNewSessionQueue(
115116
SlotMatcher slotMatcher,
116117
Duration requestTimeoutCheck,
117118
Duration requestTimeout,
119+
Duration maximumResponseDelay,
118120
Secret registrationSecret,
119121
int batchSize) {
120122
super(tracer, registrationSecret);
@@ -123,6 +125,7 @@ public LocalNewSessionQueue(
123125
Require.nonNegative("Retry period", requestTimeoutCheck);
124126

125127
this.requestTimeout = Require.positive("Request timeout", requestTimeout);
128+
this.maximumResponseDelay = Require.positive("Maximum response delay", maximumResponseDelay);
126129

127130
this.requests = new ConcurrentHashMap<>();
128131
this.queue = new ConcurrentLinkedDeque<>();
@@ -152,6 +155,7 @@ public static NewSessionQueue create(Config config) {
152155
slotMatcher,
153156
newSessionQueueOptions.getSessionRequestTimeoutPeriod(),
154157
newSessionQueueOptions.getSessionRequestTimeout(),
158+
newSessionQueueOptions.getMaximumResponseDelay(),
155159
secretOptions.getRegistrationSecret(),
156160
newSessionQueueOptions.getBatchSize());
157161
}
@@ -234,7 +238,9 @@ public HttpResponse addToQueue(SessionRequest request) {
234238
}
235239

236240
Lock writeLock = this.lock.writeLock();
237-
writeLock.lock();
241+
if (!writeLock.tryLock()) {
242+
writeLock.lock();
243+
}
238244
try {
239245
requests.remove(request.getRequestId());
240246
queue.remove(request);
@@ -268,7 +274,9 @@ Data injectIntoQueue(SessionRequest request) {
268274
Data data = new Data(request.getEnqueued());
269275

270276
Lock writeLock = lock.writeLock();
271-
writeLock.lock();
277+
if (!writeLock.tryLock()) {
278+
writeLock.lock();
279+
}
272280
try {
273281
requests.put(request.getRequestId(), data);
274282
queue.addLast(request);
@@ -288,7 +296,9 @@ public boolean retryAddToQueue(SessionRequest request) {
288296
contexts.getOrDefault(request.getRequestId(), tracer.getCurrentContext());
289297
try (Span ignored = context.createSpan("sessionqueue.retry")) {
290298
Lock writeLock = lock.writeLock();
291-
writeLock.lock();
299+
if (!writeLock.tryLock()) {
300+
writeLock.lock();
301+
}
292302
try {
293303
if (!requests.containsKey(request.getRequestId())) {
294304
return false;
@@ -324,7 +334,9 @@ public Optional<SessionRequest> remove(RequestId reqId) {
324334
Require.nonNull("Request ID", reqId);
325335

326336
Lock writeLock = lock.writeLock();
327-
writeLock.lock();
337+
if (!writeLock.tryLock()) {
338+
writeLock.lock();
339+
}
328340
try {
329341
Iterator<SessionRequest> iterator = queue.iterator();
330342
while (iterator.hasNext()) {
@@ -345,6 +357,29 @@ public Optional<SessionRequest> remove(RequestId reqId) {
345357
public List<SessionRequest> getNextAvailable(Map<Capabilities, Long> stereotypes) {
346358
Require.nonNull("Stereotypes", stereotypes);
347359

360+
// use nano time to avoid issues with a jumping clock e.g. on WSL2 or due to time-sync
361+
long started = System.nanoTime();
362+
// delay the response to avoid heavy polling via http
363+
while (maximumResponseDelay.toNanos() > System.nanoTime() - started) {
364+
Lock readLock = lock.readLock();
365+
readLock.lock();
366+
367+
try {
368+
if (!queue.isEmpty()) {
369+
break;
370+
}
371+
} finally {
372+
readLock.unlock();
373+
}
374+
375+
try {
376+
Thread.sleep(10);
377+
} catch (InterruptedException ex) {
378+
Thread.currentThread().interrupt();
379+
break;
380+
}
381+
}
382+
348383
Predicate<Capabilities> matchesStereotype =
349384
caps ->
350385
stereotypes.entrySet().stream()
@@ -360,7 +395,9 @@ public List<SessionRequest> getNextAvailable(Map<Capabilities, Long> stereotypes
360395
});
361396

362397
Lock writeLock = lock.writeLock();
363-
writeLock.lock();
398+
if (!writeLock.tryLock()) {
399+
writeLock.lock();
400+
}
364401
try {
365402
List<SessionRequest> availableRequests =
366403
queue.stream()
@@ -397,7 +434,9 @@ public boolean complete(
397434
try (Span ignored = context.createSpan("sessionqueue.completed")) {
398435
Data data;
399436
Lock writeLock = lock.writeLock();
400-
writeLock.lock();
437+
if (!writeLock.tryLock()) {
438+
writeLock.lock();
439+
}
401440
try {
402441
data = requests.remove(reqId);
403442
queue.removeIf(req -> reqId.equals(req.getRequestId()));
@@ -417,7 +456,9 @@ public boolean complete(
417456
@Override
418457
public int clearQueue() {
419458
Lock writeLock = lock.writeLock();
420-
writeLock.lock();
459+
if (!writeLock.tryLock()) {
460+
writeLock.lock();
461+
}
421462

422463
try {
423464
int size = queue.size();

‎java/test/org/openqa/selenium/grid/distributor/AddingNodesTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ public void setUpDistributor() throws MalformedURLException {
109109
new DefaultSlotMatcher(),
110110
Duration.ofSeconds(2),
111111
Duration.ofSeconds(2),
112+
Duration.ofSeconds(1),
112113
registrationSecret,
113114
5);
114115

‎java/test/org/openqa/selenium/grid/distributor/DistributorDrainingTest.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ void drainedNodeDoesNotShutDownIfNotEmpty() throws InterruptedException {
5454
new DefaultSlotMatcher(),
5555
Duration.ofSeconds(2),
5656
Duration.ofSeconds(2),
57+
Duration.ofSeconds(1),
5758
registrationSecret,
5859
5);
5960
LocalNode node =
@@ -106,6 +107,7 @@ void drainedNodeShutsDownAfterSessionsFinish() throws InterruptedException {
106107
new DefaultSlotMatcher(),
107108
Duration.ofSeconds(2),
108109
Duration.ofSeconds(2),
110+
Duration.ofSeconds(1),
109111
registrationSecret,
110112
5);
111113
LocalNode node =
@@ -182,6 +184,7 @@ void testDrainedNodeShutsDownOnceEmpty() throws InterruptedException {
182184
new DefaultSlotMatcher(),
183185
Duration.ofSeconds(2),
184186
Duration.ofSeconds(2),
187+
Duration.ofSeconds(1),
185188
registrationSecret,
186189
5);
187190
LocalNode node =
@@ -234,6 +237,7 @@ void drainingNodeDoesNotAcceptNewSessions() {
234237
new DefaultSlotMatcher(),
235238
Duration.ofSeconds(2),
236239
Duration.ofSeconds(2),
240+
Duration.ofSeconds(1),
237241
registrationSecret,
238242
5);
239243
LocalNode node =

‎java/test/org/openqa/selenium/grid/distributor/DistributorNodeAvailabilityTest.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ void shouldBeAbleToRemoveANode() throws MalformedURLException {
9797
new DefaultSlotMatcher(),
9898
Duration.ofSeconds(2),
9999
Duration.ofSeconds(2),
100+
Duration.ofSeconds(1),
100101
registrationSecret,
101102
5);
102103

@@ -147,6 +148,7 @@ void shouldIncludeHostsThatAreUpInHostList() {
147148
new DefaultSlotMatcher(),
148149
Duration.ofSeconds(2),
149150
Duration.ofSeconds(2),
151+
Duration.ofSeconds(1),
150152
registrationSecret,
151153
5);
152154
handler.addHandler(sessions);
@@ -219,6 +221,7 @@ void shouldNotRemoveNodeWhoseHealthCheckPassesBeforeThreshold() throws Interrupt
219221
new DefaultSlotMatcher(),
220222
Duration.ofSeconds(2),
221223
Duration.ofSeconds(2),
224+
Duration.ofSeconds(1),
222225
registrationSecret,
223226
5);
224227

@@ -281,6 +284,7 @@ void shouldReturnNodesThatWereDownToPoolOfNodesOnceTheyMarkTheirHealthCheckPasse
281284
new DefaultSlotMatcher(),
282285
Duration.ofSeconds(2),
283286
Duration.ofSeconds(2),
287+
Duration.ofSeconds(1),
284288
registrationSecret,
285289
5);
286290

@@ -338,6 +342,7 @@ void shouldBeAbleToAddANodeAndCreateASession() {
338342
new DefaultSlotMatcher(),
339343
Duration.ofSeconds(2),
340344
Duration.ofSeconds(2),
345+
Duration.ofSeconds(1),
341346
registrationSecret,
342347
5);
343348
LocalNode node =

‎java/test/org/openqa/selenium/grid/distributor/DistributorTest.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ void creatingASessionAddsItToTheSessionMap() {
8484
new DefaultSlotMatcher(),
8585
Duration.ofSeconds(2),
8686
Duration.ofSeconds(2),
87+
Duration.ofSeconds(1),
8788
registrationSecret,
8889
5);
8990

@@ -134,6 +135,7 @@ void shouldReleaseSlotOnceSessionEnds() {
134135
new DefaultSlotMatcher(),
135136
Duration.ofSeconds(2),
136137
Duration.ofSeconds(2),
138+
Duration.ofSeconds(1),
137139
registrationSecret,
138140
5);
139141

@@ -203,6 +205,7 @@ void shouldNotStartASessionIfTheCapabilitiesAreNotSupported() {
203205
new DefaultSlotMatcher(),
204206
Duration.ofSeconds(2),
205207
Duration.ofSeconds(2),
208+
Duration.ofSeconds(1),
206209
registrationSecret,
207210
5);
208211
handler.addHandler(sessions);
@@ -243,6 +246,7 @@ void attemptingToStartASessionWhichFailsMarksAsTheSlotAsAvailable() {
243246
new DefaultSlotMatcher(),
244247
Duration.ofSeconds(2),
245248
Duration.ofSeconds(2),
249+
Duration.ofSeconds(1),
246250
registrationSecret,
247251
5);
248252

‎java/test/org/openqa/selenium/grid/distributor/DistributorTestBase.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ public void setUp() throws URISyntaxException {
106106
new DefaultSlotMatcher(),
107107
Duration.ofSeconds(2),
108108
Duration.ofSeconds(2),
109+
Duration.ofSeconds(1),
109110
registrationSecret,
110111
5);
111112

‎java/test/org/openqa/selenium/grid/distributor/SessionSchedulingTest.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ void theMostLightlyLoadedNodeIsSelectedFirst() {
6666
new DefaultSlotMatcher(),
6767
Duration.ofSeconds(2),
6868
Duration.ofSeconds(2),
69+
Duration.ofSeconds(1),
6970
registrationSecret,
7071
5);
7172

@@ -121,6 +122,7 @@ void shouldUseLastSessionCreatedTimeAsTieBreaker() {
121122
new DefaultSlotMatcher(),
122123
Duration.ofSeconds(2),
123124
Duration.ofSeconds(2),
125+
Duration.ofSeconds(1),
124126
registrationSecret,
125127
5);
126128
Node leastRecent = createNode(caps, 5, 0);
@@ -192,6 +194,7 @@ void shouldNotScheduleAJobIfAllSlotsAreBeingUsed() {
192194
new DefaultSlotMatcher(),
193195
Duration.ofSeconds(2),
194196
Duration.ofSeconds(2),
197+
Duration.ofSeconds(1),
195198
registrationSecret,
196199
5);
197200

@@ -247,6 +250,7 @@ void shouldPrioritizeHostsWithTheMostSlotsAvailableForASessionType() {
247250
new DefaultSlotMatcher(),
248251
Duration.ofSeconds(2),
249252
Duration.ofSeconds(2),
253+
Duration.ofSeconds(1),
250254
registrationSecret,
251255
5);
252256

0 commit comments

Comments
 (0)