Skip to content

5076 Cap default Caffeine cache spec with maximumSize to bound heap#5077

Open
ivicac wants to merge 6 commits into
masterfrom
5076
Open

5076 Cap default Caffeine cache spec with maximumSize to bound heap#5077
ivicac wants to merge 6 commits into
masterfrom
5076

Conversation

@ivicac
Copy link
Copy Markdown
Contributor

@ivicac ivicac commented May 23, 2026

  • 5076 Cap polling-trigger poll loop to prevent OutOfMemoryError
  • 5076 Cap default Caffeine cache spec with maximumSize to bound heap
  • 5076 Log a warning when a polling-trigger safety cap trips

@ivicac ivicac linked an issue May 23, 2026 that may be closed by this pull request
@ivicac ivicac changed the title 5076 5076 Cap default Caffeine cache spec with maximumSize to bound heap May 23, 2026
@ivicac ivicac requested review from Copilot and igorbeslic May 23, 2026 13:59
Copy link
Copy Markdown
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

This PR adds guardrails to prevent runaway in-memory growth by (1) bounding polling-trigger “poll immediately” pagination loops and (2) adding a default maximum size to the Caffeine cache configuration, with tests to validate the behavior and a WARN log when the polling cap is hit.

Changes:

  • Add safety caps for polling-trigger pagination (max iterations + max records) and log a WARN when the cap trips.
  • Add unit tests covering capped polling behavior and cap-trip logging.
  • Cap the default Caffeine cache builder with maximumSize(1_000) to bound heap usage.

Reviewed changes

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

File Description
server/libs/platform/platform-component/platform-component-service/src/main/java/com/bytechef/platform/component/service/TriggerDefinitionServiceImpl.java Adds polling-loop safety caps and WARN logging when a cap is hit.
server/libs/platform/platform-component/platform-component-service/src/test/java/com/bytechef/platform/component/service/TriggerDefinitionServiceTest.java Adds tests for bounded polling behavior and cap-trip warning logging.
server/libs/config/cache-config/src/main/java/com/bytechef/cache/config/CacheConfiguration.java Adds a default maximumSize to the Caffeine cache builder to bound cache growth.

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

Fixes three review comments from copilot-pull-request-reviewer on PR #5077:

1. TriggerDefinitionServiceImpl: the previous loop checked records.size()
   only at loop-entry, so a runaway initial response or an addAll of a large
   page could push the accumulator past MAX_POLLING_TRIGGER_RECORDS and
   defeat the heap-safety goal. Extract appendCapped(...) that truncates
   each append to the remaining headroom, and apply it to both the initial
   pollOutput and every subsequent page. Track whether truncation occurred
   and use it (in addition to pollImmediately) as the WARN trigger so silent
   data loss can no longer pass unannounced.

2. TriggerDefinitionServiceTest.testExecutePollingTriggerStopsAfterMaxIterations:
   replace the loose <= 200 bound with <= 101 (the exact production maximum:
   100 in-loop iterations + 1 initial poll) so off-by-one regressions fail.

3. TriggerDefinitionServiceTest.testExecutePollingTriggerStopsAfterMaxRecords:
   replace the loose <= 20_000 bound with <= 10_000 (the actual record cap)
   so a record-cap overshoot can no longer slip past the test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ivicac added a commit to ivicac/bytechef that referenced this pull request May 24, 2026
Fixes three review comments from copilot-pull-request-reviewer on PR bytechefhq#5077:

1. TriggerDefinitionServiceImpl: the previous loop checked records.size()
   only at loop-entry, so a runaway initial response or an addAll of a large
   page could push the accumulator past MAX_POLLING_TRIGGER_RECORDS and
   defeat the heap-safety goal. Extract appendCapped(...) that truncates
   each append to the remaining headroom, and apply it to both the initial
   pollOutput and every subsequent page. Track whether truncation occurred
   and use it (in addition to pollImmediately) as the WARN trigger so silent
   data loss can no longer pass unannounced.

2. TriggerDefinitionServiceTest.testExecutePollingTriggerStopsAfterMaxIterations:
   replace the loose <= 200 bound with <= 101 (the exact production maximum:
   100 in-loop iterations + 1 initial poll) so off-by-one regressions fail.

3. TriggerDefinitionServiceTest.testExecutePollingTriggerStopsAfterMaxRecords:
   replace the loose <= 20_000 bound with <= 10_000 (the actual record cap)
   so a record-cap overshoot can no longer slip past the test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ivicac added a commit to ivicac/bytechef that referenced this pull request May 24, 2026
Fixes three review comments from copilot-pull-request-reviewer on PR bytechefhq#5077:

1. TriggerDefinitionServiceImpl: the previous loop checked records.size()
   only at loop-entry, so a runaway initial response or an addAll of a large
   page could push the accumulator past MAX_POLLING_TRIGGER_RECORDS and
   defeat the heap-safety goal. Extract appendCapped(...) that truncates
   each append to the remaining headroom, and apply it to both the initial
   pollOutput and every subsequent page. Track whether truncation occurred
   and use it (in addition to pollImmediately) as the WARN trigger so silent
   data loss can no longer pass unannounced.

2. TriggerDefinitionServiceTest.testExecutePollingTriggerStopsAfterMaxIterations:
   replace the loose <= 200 bound with <= 101 (the exact production maximum:
   100 in-loop iterations + 1 initial poll) so off-by-one regressions fail.

3. TriggerDefinitionServiceTest.testExecutePollingTriggerStopsAfterMaxRecords:
   replace the loose <= 20_000 bound with <= 10_000 (the actual record cap)
   so a record-cap overshoot can no longer slip past the test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
igorbeslic pushed a commit that referenced this pull request May 26, 2026
Fixes three review comments from copilot-pull-request-reviewer on PR #5077:

1. TriggerDefinitionServiceImpl: the previous loop checked records.size()
   only at loop-entry, so a runaway initial response or an addAll of a large
   page could push the accumulator past MAX_POLLING_TRIGGER_RECORDS and
   defeat the heap-safety goal. Extract appendCapped(...) that truncates
   each append to the remaining headroom, and apply it to both the initial
   pollOutput and every subsequent page. Track whether truncation occurred
   and use it (in addition to pollImmediately) as the WARN trigger so silent
   data loss can no longer pass unannounced.

2. TriggerDefinitionServiceTest.testExecutePollingTriggerStopsAfterMaxIterations:
   replace the loose <= 200 bound with <= 101 (the exact production maximum:
   100 in-loop iterations + 1 initial poll) so off-by-one regressions fail.

3. TriggerDefinitionServiceTest.testExecutePollingTriggerStopsAfterMaxRecords:
   replace the loose <= 20_000 bound with <= 10_000 (the actual record cap)
   so a record-cap overshoot can no longer slip past the test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
marko-kriskovic pushed a commit that referenced this pull request May 29, 2026
Fixes three review comments from copilot-pull-request-reviewer on PR #5077:

1. TriggerDefinitionServiceImpl: the previous loop checked records.size()
   only at loop-entry, so a runaway initial response or an addAll of a large
   page could push the accumulator past MAX_POLLING_TRIGGER_RECORDS and
   defeat the heap-safety goal. Extract appendCapped(...) that truncates
   each append to the remaining headroom, and apply it to both the initial
   pollOutput and every subsequent page. Track whether truncation occurred
   and use it (in addition to pollImmediately) as the WARN trigger so silent
   data loss can no longer pass unannounced.

2. TriggerDefinitionServiceTest.testExecutePollingTriggerStopsAfterMaxIterations:
   replace the loose <= 200 bound with <= 101 (the exact production maximum:
   100 in-loop iterations + 1 initial poll) so off-by-one regressions fail.

3. TriggerDefinitionServiceTest.testExecutePollingTriggerStopsAfterMaxRecords:
   replace the loose <= 20_000 bound with <= 10_000 (the actual record cap)
   so a record-cap overshoot can no longer slip past the test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

[bug]: Memory Leak

3 participants