Skip to content

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

Open
contrueCT wants to merge 4 commits intoapache:masterfrom
contrueCT:fix/issue-2747-load-detect-logs
Open

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

Conversation

@contrueCT
Copy link
Contributor

Purpose of the PR

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?

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@VGalaxies VGalaxies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@contrueCT
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement] Add critical log statements.

4 participants