[grid] delay the newsessionqueue response

[joerg1985]

User description

Description

This PR will delay the newsessionqueue response in case there is no data, to reduce the http requests while polling for new session requests. The maximum delay before retuning no data to the client is 8000ms. To ensure the polling does not prevent the writers to enter the lock, entering the lock is reworked too.

Motivation and Context

A single node in a fully distributed grid does perform ~50 http requests per second in my local grid.
Adding more nodes will increase the total http requests per second and does create not needed traffic.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

enhancement

---

Description

• Introduced a delay mechanism in the getNextAvailable method to reduce unnecessary HTTP requests by delaying the response for up to 8000ms if the queue is empty.
• Implemented non-blocking lock acquisition using tryLock to improve concurrency handling and reduce potential deadlocks.
• Enhanced thread handling by adding interrupt checks during sleep to ensure proper thread management.

---

Changes walkthrough 📝

Relevant files

Enhancement

LocalNewSessionQueue.java Implement delay and non-blocking locks in session queue    java/src/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueue.java Introduced a delay mechanism to reduce HTTP polling requests. Implemented a non-blocking lock acquisition using tryLock. Added a loop to delay the response for up to 8000ms if the queue is empty. Improved thread handling with interrupt checks during sleep. +43/-7

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[codiumai-pr-agent-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Thread Safety The new tryLock() pattern may not handle contention properly. If tryLock() fails, directly calling lock() could cause deadlocks if multiple threads are involved. Performance The 8 second delay with 10ms sleep intervals could cause high CPU usage. Consider using longer sleep intervals or a more efficient waiting mechanism. Resource Management The readLock is acquired inside a loop but only released if queue is empty. This could lead to lock exhaustion under high load.

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Performance	Implement exponential backoff in polling loop to reduce CPU usage and context switching The delay loop in getNextAvailable() should release the CPU more efficiently by using a longer sleep interval that gradually increases, rather than a fixed 10ms sleep which may cause unnecessary context switches. java/src/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueue.java [353-371] +long sleepTime = 50; while (8000 > System.currentTimeMillis() - started) { Lock readLock = lock.readLock(); readLock.lock(); try { if (!queue.isEmpty()) { break; } } finally { readLock.unlock(); } try { - Thread.sleep(10); + Thread.sleep(Math.min(sleepTime, 500)); + sleepTime *= 1.5; } catch (InterruptedException ex) { Thread.currentThread().interrupt(); break; } } Apply this suggestion Suggestion importance[1-10]: 9 Why: The exponential backoff strategy would significantly reduce CPU usage and system load by gradually increasing sleep intervals, while still maintaining responsiveness. This is a substantial performance improvement over the fixed 10ms sleep.	9
Best practice	Simplify lock acquisition by removing unnecessary conditional tryLock attempt The current implementation attempts to acquire a lock twice unnecessarily. If tryLock() fails, lock() is called, but if tryLock() succeeds, lock() is never called. Remove the conditional and use a single lock() call for simplicity and clarity. java/src/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueue.java [237-239] -if (!writeLock.tryLock()) { - writeLock.lock(); -} +writeLock.lock(); Apply this suggestion Suggestion importance[1-10]: 8 Why: The current implementation of trying tryLock() and then falling back to lock() is redundant and potentially confusing. Using a single lock() call would be clearer and achieve the same result with less complexity.	8
	Check thread interruption status before lock acquisition to improve responsiveness to shutdown signals The delay loop in getNextAvailable() should check for thread interruption before acquiring the lock to allow faster response to shutdown requests. java/src/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueue.java [353-355] while (8000 > System.currentTimeMillis() - started) { + if (Thread.interrupted()) { + Thread.currentThread().interrupt(); + break; + } Lock readLock = lock.readLock(); readLock.lock(); Apply this suggestion Suggestion importance[1-10]: 7 Why: Checking for thread interruption before acquiring the lock would improve system responsiveness during shutdown and prevent potential lock acquisition delays. This is a meaningful improvement for thread management and system shutdown behavior.	7

💡 Need additional feedback ? start a PR chat