Launchpad: policy panel, config load/save, OCI improvements, and UX fixes - #30
Draft
magniloquency wants to merge 51 commits into
Draft
magniloquency wants to merge 51 commits into
magniloquency wants to merge 51 commits into
Conversation
* Fix unclean shutdown leaking processes and bound ports The scaler CLI and scaler_scheduler ran the scheduler as a grandchild process that nothing signalled on shutdown: terminating the wrapper orphaned the scheduler, which kept its ports bound forever and wedged interpreter exit via the multiprocessing resource tracker. The CLI also had no SIGTERM handler (kill leaked the whole cluster), did not cover the startup window on interrupt, and joined children without a timeout. - run the scheduler in the current process via run_scheduler(); SchedulerProcess delegates to it, no nested process - scaler CLI: route SIGTERM through the Ctrl-C path, start children inside the try block, and tear down in reverse startup order with a join deadline, SIGKILL escalation, and orphaned-descendant reaping - YMQ: AcceptServer::init() returns std::expected; bind/listen failures (e.g. EADDRINUSE) propagate to the bind callback and raise a catchable SysCallError in Python instead of exiting the process uncleanly - tests: shutdown regression tests for the CLI; nested task tests use a random free port instead of a hardcoded one * Fix lint errors: mypy typing and clang-format - test_scaler_shutdown: pass the Popen between helpers instead of accessing an Optional attribute, and tighten type hints - run_scheduler: shutdown callback returns None as the handler expects - apply clang-format-21 to the new AcceptServer::init call sites * Fix mypy on Windows: guard POSIX-only process group kill os.killpg, os.getpgid and signal.SIGKILL do not exist on Windows, and mypy checks the module there even though the test class is skipped. * Fix macOS CI: avoid psutil.net_connections (needs root on macOS) The shutdown regression test runs on macOS (only win32 is skipped), but the Python test suite runs without sudo there, and psutil.net_connections() requires root on macOS -- it raised AccessDenied and failed the test. Probe ports with socket.create_connection() instead (the same idiom ObjectStorageServerProcess.wait_until_ready uses), which needs no privileges on any platform. Also harden the process-tree helpers against psutil.AccessDenied so teardown and the failure message cannot raise. --------- Co-authored-by: sharpener6 <1sc2l4qi@duck.com>
…inos#833) * Waterfall policy: make max_task_concurrency optional in rule format When omitted from the CSV policy_content, the rule's max_task_concurrency is None and the policy falls back to the value reported by the worker manager heartbeat. An explicit value still acts as a soft cap (min of rule cap and heartbeat cap). * style: wrap long ternary in waterfall scaling policy * Update version.txt Signed-off-by: magniloquency <197707854+magniloquency@users.noreply.github.com> --------- Signed-off-by: magniloquency <197707854+magniloquency@users.noreply.github.com>
…g policies Policies now unconditionally emit a setDesired command so the worker manager always receives the authoritative desired count, even when it matches the current worker count.
- Add Policy pane with custom dropdown: Load Balancer (simple), Waterfall (waterfall_v1), and a disabled Lowest Cost (greedy) option - Retitle Scheduler panel to "Scheduler (AWS-only)" - Add "Load config.toml" button alongside the existing download button; uses @ltd/j-toml (UMD CDN) to parse and hydrate all form state - Emit policy_content in config.toml when waterfall_v1 is selected, deriving priority from worker manager list order; restore order on load - Make worker managers draggable via grab handles (SVG dot-grid icon); drop reorders the array and updates waterfall priority badges - Show numbered priority badges on WM tabs when waterfall policy is active - Show hint text under the policy dropdown when waterfall is selected
Without this, the field defaulted to os.cpu_count() on the Launchpad host, causing the worker manager to spin up more instances than configured and report a higher max concurrency than expected.
Allows scrolling back through the entire log file. Note: SSM GetCommandInvocation caps StandardOutputContent at 48,000 chars, so very large log files will still be truncated at that limit.
buildConfigToml and buildUserData had separately hand-rolled, drifting TOML templates; buildUserData was missing the [scheduler.policy] section entirely, silently dropping waterfall_v1 policy selection on real EC2 deploys. Both now build through shared buildWorkerManagerTable/buildSchedulerConfigToml helpers and serialize via @ltd/j-toml's TOML.stringify instead of string concatenation.
…g policies Policies now unconditionally emit a setDesired command so the worker manager always receives the authoritative desired count, even when it matches the current worker count.
Adds a sessionStorage-backed hot-reload dev mode activated via ?dev URL parameter. Credentials are restored from sessionStorage on mount and saved on every change, surviving page reloads within the same tab but cleared when the tab closes. A small DEV badge appears in the nav bar when the flag is active. The existing "not stored by this application" disclaimer remains accurate for normal usage.
Policies no longer skip emission when desired equals current, so tests that expected [] in those cases now assert the emitted command carries the correct desired count. One test made redundant by the change (test_balanced_ratio_emits_current, duplicate of test_no_action_when_ratio_is_in_band) is removed.
VanillaScalingPolicy: don't scale up in the low-ratio branch when the manager has no connected workers (current == 0). The low-ratio branch is a drain branch; the max(1, ...) floor was intended to prevent a running manager from fully draining while tasks remain, not to bootstrap a new one. When another manager's workers cover the load, the ratio can momentarily dip below 1, causing spurious scale-up and repeated start/stop cycles on the EC2 manager before it ever connects. Launchpad: include the `policy` field in the deployment cfg object. It was present in the download-config path but missing from the provisioning path, causing buildSchedulerConfigToml to always fall back to policy_engine_type = "simple" in the EC2 user data regardless of the UI selection.
pip failed to clone the scaler package from GitHub because git was not installed on the instance.
- Switch from python -m venv + pip to uv (matches Launchpad provisioner) - Drop dnf python packages; uv manages the Python version - For git installs: install gcc14, build capnp from source via library_tool.sh, ldconfig /usr/local/lib, pass CMAKE_ARGS so scikit-build-core uses gcc14 and shared libuv - For PyPI wheel installs: no C++ deps needed, uv only
* feat: zero-copy Python buffer support in YMQ (finos#789) Split the monolithic concrete `Bytes` class into an abstract base (`Bytes`) and two concrete implementations: `BufferedBytes` (C++-owned heap memory) and `PyBufferBytes` (zero-copy Python buffer that holds a `Py_buffer` view and acquires the GIL only on destruction). All YMQ socket and message APIs now use `std::unique_ptr<Bytes>` for ownership, eliminating the O(message-size) copy that previously occurred when Python passed a buffer to `PyBytes_init`. * fix: eliminate double GIL acquisition in PyBufferBytes destructor on send completion SendMessageCallback now receives ownership of the sent payload so the pymod lambdas can destroy it inside their existing AcquireGIL scope, reducing GIL acquisitions on the libuv thread from two per send to one. Covers all three problem sites: MessageConnection's send-completion lambda, ConnectorSocket's disconnected fast-path, and BinderSocket's shutdown pending-send loop. * Merge rafa/scaler-timeout-fixes: drop binder sends to disconnected peers and fix Capnp __getattr__ stack overflow * fix: update rafa's new SendMessageCallback call sites to pass buffer as second argument Rafa's scaler-timeout-fixes branch added two error-path invocations of SendMessageCallback using the old one-argument signature. Our zero-copy changes extended the signature to two arguments; update those sites to match. * Add end-to-end zero-copy integration test for YMQ Bytes and Cap'n Proto Verifies that Bytes(buf) and Message.from_bytes() share the same underlying buffer so mutations to buf are visible through a deserialized Cap'n Proto message without any copies. Also adds .envrc to .gitignore and commits uv.lock. * ymq: rename as_string to asString, move to bytes.h, fix C-style casts - Rename free function as_string -> asString to match project camelCase convention - Move asString(const Bytes&) from buffered_bytes.h to bytes.h so it is available on the abstract interface without including the concrete header - Remove redundant BufferedBytes::as_string() member (covered by the free function) - Replace C-style casts with reinterpret_cast in object_storage_server.h writeMessage - Add comment on shared_ptr(unique_ptr&&) in ObjectManager::setObject explaining the two-allocation cost vs make_shared * gitignore: move .envrc to local git exclude * ymq: fix asString noexcept, guard memcpy on zero size, remove dead is_null - asString was incorrectly noexcept; std::string ctor can throw bad_alloc - BufferedBytes(const char*, size_t) called memcpy with a potentially null src when size==0, which is UB; guard with size > 0 check - Remove unused BufferedBytes::is_null() (also violated camelCase convention) - Restore // Send OSS header comment in writeMessage * Bump version from 2.4.5 to 2.4.6 Signed-off-by: magniloquency <197707854+magniloquency@users.noreply.github.com> * review: const-qualify local vars, remove dead common.h Address PR finos#792 review comments from gxuu. * ymq: remove common.h from CMakeLists * review: delete Bytes move ctor/assignment, fix derived class moves Bytes(Bytes&&) and operator=(Bytes&&) are deleted to enforce the pure-interface contract — the base has no data to move and polymorphic moves of the base subobject are not meaningful. Derived classes (BufferedBytes, PyBufferBytes) replace their =default move operations with explicit implementations that no longer chain to the now-deleted base moves. BufferedBytes also zeroes _size on the moved-from object, which =default was leaving inconsistent (nullptr data but non-zero size). * ymq: make Bytes special members default, simplify subclass moves ~Bytes() is no longer pure virtual; move/copy ctor and assignment on Bytes are defaulted instead of deleted. Derived classes (BufferedBytes, PyBufferBytes) drop their now-redundant custom move implementations in favour of = default, per project style. * ymq: make asString a virtual method on Bytes Promotes the free function asString(const Bytes&) to a pure virtual method on Bytes, with implementations in BufferedBytes and PyBufferBytes. Updates all call sites across src, tests, and examples. message_connection.cpp adopts asString() for identity parsing. * Bump version from 2.5.2 to 2.5.3 Signed-off-by: magniloquency <197707854+magniloquency@users.noreply.github.com> --------- Signed-off-by: magniloquency <197707854+magniloquency@users.noreply.github.com>
* Unify log format across Python and C++ components (finos#691) Change log output from `[INFO]2026-04-08 09:58:52-0400: ...` to `2026-04-08 09:58:52-0400 INFO scheduler[23451]: ...` so operators can identify which subprocess (scheduler / worker_manager / object_storage_server / gui) emitted each line via the `<process_name>[<pid>]` prefix. Python side: `setup_logger` now takes `process_name` and bakes it into the formatter; all 6 CLI entry points pass their component name. C++ side: `scaler::ymq::Logger` gains a `name` constructor arg and supports `%(name)s` / `%(process)d` tokens so in-house C++ daemons (ymq, object_storage) emit the same format. Signed-off-by: gxu <georgexu420@163.com> * Migrate to named loggers to avoid mutating the root logger (finos#653) Library code should not reconfigure the root logger — host applications (or third-party SDKs such as ORB) may configure root independently and would silently override or suppress scaler output. Key changes: - `scaler/__init__.py` attaches a `NullHandler` to the "scaler" logger so library imports (e.g. `from scaler import Client`) don't emit "no handler" warnings or touch root. - `setup_logger` now configures the "scaler" logger with `propagate=False` (instead of root). Daemon entry points still get consistent formatting; submodule loggers inherit via the `scaler.*` namespace. - Every module uses `logger = logging.getLogger(__name__)` and emits through that logger. `logging.<method>()` module-level calls were all migrated. - `get_logger_info(logging.getLogger("scaler"))` reads scaler's effective format/level/paths for the C++ object storage server. - Removed the ORB "setup_logger after __aenter__" workaround: ORB can no longer clobber scaler's logging now that we no longer share the root logger. Signed-off-by: gxu <georgexu420@163.com> Format Signed-off-by: gxu <georgexu420@163.com> Format Signed-off-by: gxu <georgexu420@163.com> Fix pflake Signed-off-by: gxu <georgexu420@163.com> S Signed-off-by: gxu <georgexu420@163.com> S Signed-off-by: gxu <georgexu420@163.com> * Address comments Signed-off-by: gxu <georgexu420@163.com> * Reduce redundant syscalls Signed-off-by: gxu <georgexu420@163.com> * More logging to logger change Signed-off-by: gxu <georgexu420@163.com> * Bump version number Signed-off-by: gxu <georgexu420@163.com> * Fix test Signed-off-by: gxu <georgexu420@163.com> --------- Signed-off-by: gxu <georgexu420@163.com> Co-authored-by: gxu <georgexu420@163.com> Co-authored-by: magniloquency <197707854+magniloquency@users.noreply.github.com>
The test was asserting 'python3.13' and 'dnf install', reflecting the old dnf-based Python install. Since commit 12fadff, the non-git auto-install branch uses 'uv venv --python 3.13' instead.
* fix issue of lacking __repr__ Signed-off-by: gxu <georgexu420@163.com> * Bump version number Signed-off-by: gxu <georgexu420@163.com> * Wire up capnp_struct_repr to bootstrap Signed-off-by: gxu <georgexu420@163.com> * Move static const vars up Signed-off-by: gxu <georgexu420@163.com> --------- Signed-off-by: gxu <georgexu420@163.com> Co-authored-by: gxu <georgexu420@163.com>
The Python scheduler reads policy_engine_type/policy_content as flat keys under [scheduler], but the provisioner was emitting them as [scheduler.policy], which the parser silently drops — causing Waterfall policy to never apply.
The ORB worker manager derives max_instances = ceil(max_task_concurrency / vCPUs_per_instance), so writing the instance count directly halved the actual number of instances provisioned. Mirror the oci_raw approach: multiply by inst.vcpu on write and divide back on load. Throw immediately if the instance type is not found in SCALER_INSTANCES rather than silently defaulting.
Seed ociMemoryGb and ociOcpus explicitly in the oci_raw onChange so there is one source of truth. Remove the stale || 30 / || 4 fallbacks from provisioner.js; configFromToml now throws if these fields are absent rather than silently provisioning the wrong capacity.
The old implementation used rfind("@") which mis-split ssh user@host and auth
token URLs, and matched commented-out lines via find("git+"). Rewrite using
Requirement for PEP 508 parsing (env markers, comment skipping) and urlsplit
to find the @Branch separator only in the path component, not the authority.
Adds unit tests covering https, ssh user@host, token auth, env markers, and
comments.
Moved ID generation into the setWorkerManagers updater so it can check against the live list and bump the counter until the candidate ID is free. Removed the wmCounterRef reset on config load — collision detection makes it unnecessary. Added a unique-ID check to the pre-launch validation so duplicate IDs block launch with a visible error.
… load Previously network_backend was omitted from the TOML for zmq, making round-tripping impossible. Now it is always written explicitly as "ymq" or "zmq" in the orb_aws_ec2 worker manager section. The scheduler env var is also set unconditionally, fixing a tcp_zmq typo (valid enum name is zmq). configFromToml reads network_backend from the first worker manager entry and handleLoadConfig restores it via setNetBack.
Instead of cat-ing the whole log file (truncated from the head by SSM), track a byte offset and fetch tail -c +$((offset+1)) each poll. On new content, advance the offset by bytes received and loop immediately for the next chunk. Once output is empty we are caught up and the normal interval timer resumes. Offset resets to 0 on each new polling session so the full log is always read from the top.
Interpolating unsanitized values into a shell script that runs as root at boot is unsafe. Use shlex.quote on both values.
configFromToml was not reading ecs_python_requirements back from the TOML, leaving the requirements textarea empty after a load and dropping the field on re-download.
Split the single useEffect into a reset effect (runs on instanceId/credential change) and a polling effect (runs on isActive/intervalMs change). Previously, the reset of byteOffsetRef and lines was bundled with the polling start, so returning to the logs tab always re-fetched from byte 0.
Adds a visible warning banner in the credentials pane and a tooltip on the DEV badge noting that browsers may flush sessionStorage to disk via crash-recovery, so plaintext secrets can outlive a tab close. Also adds a code comment at the sessionStorage effect for the same reason.
…offset inflation credentials was passed as a new object literal on every App render, and setWorkerMonitorElapsed re-rendered App every second. This caused fetchLogs (whose useCallback depended on credentials) to be recreated every second, which restarted the polling useEffect every second. When a new polling cycle started while fetchLogs was mid-flight awaiting the SSM response, both the old and new calls completed with the same byte offset and both appended the same lines (duplicate log entries) and both incremented byteOffsetRef by the same amount (inflated offset). Once the offset exceeded the actual file size, tail -c +N returned empty and the log appeared to stop refreshing. Fix: hold credentials in a ref updated unconditionally on each render, and remove credentials from fetchLogs' useCallback deps. fetchLogs identity is now stable across the parent's frequent re-renders.
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
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.
Mirror of finos#836. Base is
coordinator-heartbeat-desired-count; this PR shows only these changes.Summary
waterfall_v1andsimplepolicies in the Launchpad UI.Test plan
policy_contentin the generated TOML