Fix unbounded session table growth in directory#614
Merged
amrc-benmorrow merged 6 commits intomainfrom Mar 26, 2026
Merged
Conversation
The Directory service uses Set<Address> for tracking online devices, but JavaScript Sets use reference equality. By caching Address instances in a static Map keyed by their string representation, we ensure only one object exists per unique group/node/device combination, making Set.has() work correctly.
Add cleanup_old_sessions query that deletes all non-current sessions for a device, and call it from on_session_notify after the change-notify has been published. This prevents unbounded accumulation of historical session rows (which reached 11M in production) while preserving the previous session long enough to compute the schema diff. Only the current session's notify handler triggers cleanup, since it runs after the old session's schemas have been read.
Test that the Address class constructor cache returns the same instance for identical group/node/device combinations, including via parse() and parent_node(). Verify Set compatibility and null/empty device normalisation.
Also add long as an explicit dependency — it was imported directly by mqttcli.js but only available transitively via sparkplug-payload.
amrc-benmorrow
previously approved these changes
Mar 25, 2026
Contributor
amrc-benmorrow
left a comment
amrc-benmorrow
left a comment
There was a problem hiding this comment.
I'm not 100% sure about the interning but I've approved anyway.
Address interning is a potential memory leak if a service creates many distinct addresses. Instead, use addr.toString() for the Set<string> operations on this.online in the directory.
amrc-benmorrow
approved these changes
Mar 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
new Address("G","N","D") === new Address("G","N","D"). This fixesthis.onlineSet operations in the directory's MQTT client, which were silently broken because JavaScript Sets use reference equality for objects.schema_used). Nowon_session_notifydeletes all non-current sessions for a device once it's finished computing the schema diff. Existing installs clean up organically as devices rebirth.acs-directoryandjs-service-clientwith 15 tests covering Address interning, session cleanup queries, andfind_schemasmetric tree traversal.Test plan
cd lib/js-service-client && npm test— 8 Address interning tests passcd acs-directory && npm test— 7 tests pass (session cleanup + find_schemas)