Skip to content

Bound runtime diagnostics and log retention; stop failed HTTP streams - #430

Closed
guido4096 wants to merge 5 commits into
danielkucera:masterfrom
guido4096:fix/runtime-performance
Closed

guido4096 wants to merge 5 commits into
danielkucera:masterfrom
guido4096:fix/runtime-performance

Conversation

@guido4096

@guido4096 guido4096 commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

Superseded by five single-commit PRs

Split at the maintainer's request. The replacement PRs each target master directly and contain one original commit:

Part Scope PR / branch
1/5 Correct host-test mocks #431
2/5 Bound Logs page retention #432
3/5 Schedule diagnostics by elapsed time #433
4/5 Protect string-pool IDs and concurrent access #434
5/5 Buffer HTTP streams and stop after send failures #435

Each replacement has its own scope, standalone host-test results and remaining validation notes. The five extracted patches reconstruct this PR's exact tracked file tree. Suggested merge order: host-test mocks first, then the remaining fixes independently.

This combined PR is closed in favor of the five linked replacements.


Original combined description (historical)

I noticed the adapter was becoming less responsive over time. This PR addresses four concrete sources of growing or unnecessary work: retained browser log keys, diagnostics triggered by every notification, string-pool overflow/concurrent access, and repeated HTTP sends after a socket failure.

The changes are split into five commits so each fix can be reviewed independently.

Commits

1. 706fcef — Correct host-test mocks

Use the host's real stat definition and filesystem behavior for persistence tests, rather than a fabricated structure and an always-failing stub. Move C++ standard headers outside the FreeRTOS mock's extern "C" block.

This is test infrastructure only. It makes persistence checks trustworthy and prevents include-order-dependent compilation problems; it does not change ESP firmware behavior.

2. c8e7374 — Keep the Logs page bounded

The duplicate-key set previously retained keys after their log entries were evicted. At the retention limit, the page also rebuilt its entire log display repeatedly.

Keep duplicate keys aligned with retained entries and remove evicted DOM rows incrementally. Regression coverage exercises prolonged retention, filtering, clearing and restart markers.

3. 622b335 — Limit diagnostics to their intended schedule

Bus/log notifications woke the monitor and triggered expensive heap/socket inspection, even before its nominal 30-second interval elapsed. Counting these wakes also made the “hourly” heap trend run too often.

Use monotonic deadlines: health every 30 seconds, summaries every five minutes, and trend samples initially and then hourly. Collect heap and socket information outside the critical section and correct uptime_seconds to use seconds. Tests verify that frequent notifications cannot accelerate sampling.

4. 2a0558d — Protect string-pool IDs and concurrent access

The 8-bit entry counter could wrap when adding a 256th string, invalidating existing IDs. Concurrent insertions were also unsynchronized.

Limit the pool to 255 addressable entries, reject names longer than its length representation, and synchronize insertion/lookup. Existing strings remain accessible when either entry or byte capacity is exhausted. Tests cover both limits, oversized names and concurrent interning.

5. aa4baa2 — Stop failed HTTP streams promptly

Streaming endpoints previously sent many small fragments and continued attempting writes after a send failure, potentially repeating socket waits.

Add a shared 1024-byte chunk buffer, stop socket output after the first error, and return that error to the HTTP server. Apply it to streaming endpoints and finish configuration-drift JSON before terminating the response. Reduce send/receive waits from 10/10 seconds to 3/5 seconds. Tests check preserved payloads, response termination and failure propagation without repeated sends.

Validation completed

  • All 180 host tests passed on the final five-commit branch.
  • The ESP32-C3 internal firmware built successfully before the commit split; the final tracked file tree was verified identical to that tested implementation.
  • Whitespace checks passed.
  • No on-device soak-test result or measured performance improvement is claimed yet. This PR remains a draft pending hardware checks and CI.

Hardware checks before marking ready

  • Sustained operation: run on the actual installation for at least 24 hours, ideally 48, with normal eBUS traffic and MQTT/HTTP use. Record uptime, response times, free heap and largest free block after startup and periodically. Check for resets, progressively slower responses or a sustained downward heap trend. Minimum-ever free heap alone is not evidence of a leak.
  • Logs page: leave it open long enough to pass 2000 entries repeatedly. Confirm scrolling/filtering/clearing still work, recent messages keep arriving, and browser memory settles rather than growing continuously.
  • Disconnected clients: cancel a large log/config/metrics response or disconnect its client, then reconnect and request another page. Confirm the server recovers promptly and normal reads continue.
  • Diagnostics: confirm uptime advances in seconds, health refreshes approximately every 30 seconds, and frequent traffic does not fill the hourly trend with rapid samples.
  • Normal operations: verify command reads/writes, MQTT updates, configuration save/reboot, command import/export and a normal firmware upload. Specifically check slower Wi-Fi clients and uploads because the socket timeouts are shorter.
  • CI: confirm the host suite and both firmware build variants pass.

Please record firmware revision, hardware type, test duration and any observed resets or memory/latency trends. The main hardware review points are the shorter HTTP timeouts, the additional HTTP buffer's stack use, and synchronization of the shared string pool.

@yuhu-

yuhu- commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator

I assume you'll perform the suggested tests on your adapter, correct?

@guido4096

Copy link
Copy Markdown
Collaborator Author

yes, doing that now

Keep duplicate keys in sync with retained entries and remove only evicted DOM rows. Cover long-running retention, filtering and restart markers.
Throttle heap and socket sampling independently of notification frequency, collect expensive data outside critical sections, and report uptime in seconds. Test sampling and reporting deadlines.
Bound IDs and string lengths, synchronize access, and retain existing IDs at capacity. Cover exhaustion and concurrent access.
Share bounded chunk buffering across streaming endpoints, return send errors without retrying, and shorten socket waits. Finish drift JSON before terminating its response. Test payload preservation, failed sends and valid response termination.
@guido4096
guido4096 force-pushed the fix/runtime-performance branch from aa4baa2 to 50a798d Compare October 31, 2026 18:40
@danielkucera

Copy link
Copy Markdown
Owner

Please split into separate PRs.

@guido4096

Copy link
Copy Markdown
Collaborator Author

Please split into separate PRs.

done

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.

3 participants