optimize(api): add warning logs for load-based request rejection by contrueCT · Pull Request #2972 · apache/hugegraph

contrueCT · 2026-03-19T07:54:20Z

Purpose of the PR

close [Improvement] Add critical log statements. #2747

Main Changes

Log busy-worker and low-memory rejections in LoadDetectFilter before returning ServiceUnavailableException so operators can diagnose 503 responses from server-side logs.

Add unit coverage for whitelist bypass, busy rejection,
low-memory rejection, and healthy request pass-through, and register the new test in UnitTestSuite.

Verifying these changes

Trivial rework / code cleanup without any test coverage. (No Need)
Already covered by existing tests, such as (please modify tests here).
Need tests and can be verified as follows:
- xxx

Does this PR potentially affect the following parts?

Dependencies (add/update license info & regenerate_known_dependencies.sh)
Modify configurations
The public API
Other affects (typed here)
Nope

Documentation Status

Doc - TODO
Doc - Done
Doc - No Need

Copilot

Pull request overview

Adds operator-visible warning logs when LoadDetectFilter rejects requests due to high worker load or low free memory (503s), and introduces unit tests to cover the new rejection behavior plus whitelist/healthy pass-through.

Changes:

Add WARN logging for worker-load and low-memory request rejections in LoadDetectFilter.
Add LoadDetectFilterTest unit coverage for whitelist bypass, busy rejection, low-memory rejection, and healthy requests.
Register the new unit test in UnitTestSuite.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java	Adds rejection `WARN` logs and minor refactor (captures current load).
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/LoadDetectFilterTest.java	New unit tests for whitelist/busy/low-memory/healthy scenarios.
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java	Adds `LoadDetectFilterTest` to the unit suite.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...aph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java

.../hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/LoadDetectFilterTest.java

...aph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.../hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/LoadDetectFilterTest.java

...aph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java

.../hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/LoadDetectFilterTest.java

VGalaxies

Thanks for the update. I left two small review comments; CI looks green on my side, but I think these are still worth addressing.

...aph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java

.../hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/LoadDetectFilterTest.java

contrueCT · 2026-03-23T14:25:34Z

Thanks for the update. I left two small review comments; CI looks green on my side, but I think these are still worth addressing.

⚠️ The busy-load branch is rate-limited, but this low-memory WARN will still fire on every rejected request. Under sustained memory pressure that can flood the logs and add more I/O pressure to an already stressed server. Please apply the same REJECT_LOG_RATE_LIMITER here, or route both rejection logs through a shared sampling helper.
⚠️ This test drives the real System.gc() path by forcing the low-memory branch. That makes the suite slower and can make it behave differently on constrained CI nodes or JVMs with different GC ergonomics. Consider adding a test seam for the GC step so the test can stub it and only verify the rejection behavior.

Changes made:

Applied the same reject-log rate limiting to the low-memory branch, and routed both rejection paths through a shared warning helper so the sampling behavior is consistent.
Added a small test seam for gcIfNeeded() / reject-log sampling so the unit test no longer depends on a real System.gc() call.
Expanded LoadDetectFilterTest to verify:
- busy rejection emits a warning log
- low-memory rejection emits a warning log
- whitelist / healthy paths do not emit rejection logs
- reject-log sampling suppresses repeated warning logs as expected

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T03:30:02Z

.../hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/LoadDetectFilterTest.java

+    private static final Logger TEST_LOGGER =
+            (Logger) LogManager.getLogger(LoadDetectFilter.class);
+


TEST_LOGGER is obtained by casting LogManager.getLogger(LoadDetectFilter.class) to org.apache.logging.log4j.core.Logger. With the test log4j2.xml using <AsyncLogger name="org.apache.hugegraph" ...>, this logger instance can be an AsyncLogger, so this cast (and addAppender/removeAppender) can fail with ClassCastException or not attach the appender as intended. Consider attaching the TestAppender via LoggerContext/Configuration/LoggerConfig (e.g., config.getLoggerConfig(loggerName).addAppender(...) + ctx.updateLoggers()), and remove it the same way in teardown.

Copilot · 2026-03-24T03:30:03Z

...aph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java

+            boolean gcTriggered = this.gcIfNeeded();
+            long allocatedMemAfterCheck = Runtime.getRuntime().totalMemory() -
+                                          Runtime.getRuntime().freeMemory();
+            long recheckedFreeMem = (Runtime.getRuntime().maxMemory() -
+                                     allocatedMemAfterCheck) / Bytes.MB;
+            this.logRejectWarning("Rejected request due to low free memory, method={}, path={}, " +
+                                  "presumableFreeMemMB={}, recheckedFreeMemMB={}, gcTriggered={}, " +
+                                  "minFreeMemoryMB={}",
+                                  context.getMethod(), context.getUriInfo().getPath(),
+                                  presumableFreeMem, recheckedFreeMem, gcTriggered,
+                                  minFreeMemory);


In the low-free-memory rejection path, allocatedMemAfterCheck / recheckedFreeMem are computed on every rejected request, even when the reject log is suppressed by REJECT_LOG_RATE_LIMITER (since allowRejectLog() is only checked inside logRejectWarning()). To keep the rate-limiting effective under overload/low-memory conditions, consider checking allowRejectLog() first (or having logRejectWarning() return a boolean) and only doing the recheck computations when a log will actually be emitted.

contrueCT added 2 commits March 19, 2026 15:50

fix(api): add load rejection warning logs

f24f3fa

fix(api): style check

c72f2f4

imbajin requested a review from Copilot March 20, 2026 09:03

Copilot started reviewing on behalf of imbajin March 20, 2026 09:07 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

...aph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java Outdated Show resolved Hide resolved

.../hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/LoadDetectFilterTest.java Outdated Show resolved Hide resolved

imbajin reviewed Mar 20, 2026

View reviewed changes

...aph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java Outdated Show resolved Hide resolved

fix(filter): enhance load detection logging and memory management

38d8a66

imbajin requested a review from Copilot March 23, 2026 05:57

Copilot started reviewing on behalf of imbajin March 23, 2026 05:57 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

.../hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/LoadDetectFilterTest.java Show resolved Hide resolved

...aph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java Outdated Show resolved Hide resolved

imbajin reviewed Mar 23, 2026

View reviewed changes

...aph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java Outdated Show resolved Hide resolved

imbajin reviewed Mar 23, 2026

View reviewed changes

.../hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/LoadDetectFilterTest.java Show resolved Hide resolved

VGalaxies reviewed Mar 23, 2026

View reviewed changes

...aph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java Outdated Show resolved Hide resolved

.../hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/LoadDetectFilterTest.java Show resolved Hide resolved

fix(api): harden load rejection logging

d5d7ce0

imbajin requested a review from Copilot March 24, 2026 03:26

Copilot started reviewing on behalf of imbajin March 24, 2026 03:26 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

optimize(api): add warning logs for load-based request rejection#2972

optimize(api): add warning logs for load-based request rejection#2972
contrueCT wants to merge 4 commits intoapache:masterfrom
contrueCT:fix/issue-2747-load-detect-logs

contrueCT commented Mar 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

VGalaxies left a comment

Uh oh!

Uh oh!

Uh oh!

contrueCT commented Mar 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		private static final Logger TEST_LOGGER =
		(Logger) LogManager.getLogger(LoadDetectFilter.class);

Conversation

contrueCT commented Mar 19, 2026

Purpose of the PR

Main Changes

Verifying these changes

Does this PR potentially affect the following parts?

Documentation Status

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

VGalaxies left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

contrueCT commented Mar 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants