# SDP logging and correctness review — 2026-10-16 The initial logging fixes, confirmed bug fixes, requested journal archiving, and first-time rsync host trust were submitted in **Perforce changelist 34003**. Earlier requested behavior, including `NumJnls > 2`, is preserved. The user subsequently approved the improvement recommendations, except any change to intentional `server.locks` removal. That second batch is implemented and remains unsubmitted; this work has not deployed the pending candidate to production. **Windows removal:** the user confirmed that Windows support should remain removed. Both Windows scripts were deleted in changelist 34003. Their seven obsolete regression cases and PowerShell test discovery have also been removed. The Windows fixes and skipped-test counts described in the historical validation below predate that test cleanup and are not part of the current suite. **Shell execution behavior:** all newly added `set -e`, `pipefail`, strict-mode combinations, and `bash -e` invocations were removed at the user's direction. Existing depot settings are preserved. The affected scripts use explicit command-status checks to retain the logging and bug fixes without introducing global shell-option changes. ## Scope and baseline For the expanded production assurance review, the operator confirmed that the fleet uses the workspace `sdp.tgz`, not an assumed Perforce changelist. The archive is preserved unchanged with SHA-256 `1bd23217356ee3808e040d39aea5be9519bda7c8f2b4924656a70aff8ad7293a`. A separate export supplies the 189 non-Windows files for paired comparisons; its two unused Windows members are excluded. The fleet contains more than 250 machines and p4d releases 2023.1 through 2026.1, with commit, edge, standby, broker and proxy roles, independent storage and live journal `/p4/1/journals/journal`. See [production validation](production_validation.md) for required evidence and proposed rollout review. Individual host drift, patch builds and CPU architectures have not been inventoried here. Reviewed the maintained SDP source tree: Unix operational scripts and shared helpers, Maintenance programs, setup and service templates, triggers, Windows checkpoint handling, AWS snapshot tooling, binary downloads, configuration examples, and test/documentation context. Reviews combined source inspection, depot comparisons, static checks, and focused offline regressions. This is not production validation of every topology or external integration. Several files had local contents different from `p4 print #have` despite matching have/head revision metadata; other active depot files were absent locally. For changed files with that discrepancy, fixes were applied to the saved depot baseline. Existing requested edits were preserved. Active missing files were inspected from depot copies; only the snapshot and downloader scripts were restored locally for their fixes. Deleted depot files still present locally and vendored `get-pip.py` were excluded from maintained-code changes. The user subsequently requested a force sync of unopened files across the `rjackson_macmini` workspace. That refreshed 633 modified, missing, or outdated unopened files, including the remaining SDP discrepancies. Verification found no unopened tracked content mismatches, missing files, or revisions behind head. All 97 pending edits, additions, and deletions retained their actions and file contents during the sync. Existing local copies were backed up outside the workspace. Baseline comparisons establish that the bytes differed, not why the differences occurred. ## Applied fixes ### Backup, recovery, and runtime scripts - **Backup retention matched the wrong numbers.** Patterns ending in `.jnl.1*` or `.ckp.1*` also selected `10` and `100`. Unix and Windows cleanup now require the full artifact number before a suffix. Deletion failures are retained and reported. Windows journal retention counts distinct journal numbers, not their checksum sidecars. - **Recovery could continue after a failed preparation.** Edge recovery checks checkpoint restoration, edge dump/replay, database moves, and offline rebuilds. Failed preparation stops before deleting the live database. Failover reads the final journal-copy state after stopping p4d, validates the selected journal before identity changes, and does not start p4d after a failed replay. These changes improve failure handling; they do not make the entire multi-step recovery transaction reversible. - **Restore selection could choose unrelated checksums.** Offline restore and `load_checkpoint.sh -latest` restrict candidates to the checkpoint prefix instead of treating every `.md5` file as a checkpoint. Source existence is checked before removing offline databases. This is an existence/preflight check, not a new full-integrity verification process. - **Failed preflights looked safe.** Failed journal-counter, readonly-directory configuration, rsync-statistics, and filesystem-capacity queries now stop the dependent operation and retain diagnostics. Rsync receives literal argument paths instead of executing constructed shell text with `eval`. Journal-watch thresholds accept plain bytes and valid units/percentages, and reject malformed values. - **Cache cleanup accepted disabled shelf promotion.** `cacheclean.sh` requires `dm.shelve.promote=1` using the configured `P4BIN`. Failed queries and individual deletion-worker failures produce failure status. Counts reflect successful deletions. - **Setup helpers left password files after failure.** The four edge/standby helpers pipe passwords to `p4 passwd`; they no longer need temporary plaintext password files or overwrite unrelated `passwd.txt` files. - **Diagnostics were incomplete.** Replica synchronization retains both attempts' output; daily/rotate events use the correct offline journal-number variable. `ccheck.sh` reports a failed logging-library load before its local error helper is defined. - **User-requested first-time rsync connections.** The four maintained remote-transfer scripts default `RSYNC_RSH` to `ssh -o StrictHostKeyChecking=accept-new`, including retries and generated transfers. With OpenSSH 7.6+, new host keys are accepted and saved automatically while changed known keys are rejected. Explicit custom transports take precedence; unattended authentication still requires SSH credentials. Local-only copies are unchanged. Relevant implementation: [backup functions](../Server/Unix/p4/common/bin/backup_functions.sh), [edge recovery](../Server/Unix/p4/common/bin/recover_edge.sh), [standby promotion](../Server/Unix/p4/common/bin/convert_standby_to_master.sh), [checkpoint loading](../Server/Unix/p4/common/bin/load_checkpoint.sh), [cache cleanup](../Server/Unix/p4/common/bin/cacheclean.sh). ### Maintenance programs - Added durable per-script text diagnostics, start/completion records, and failure records through `sdputils.maintenance_run`. Console output remains available. Command failures retain diagnostics without including command argument lists that may contain secrets. - Weekly wrappers stop after authentication failure, preserve step failures, return nonzero on failed work, and do not use a stale client report after report generation fails. - P4Python helpers no longer translate failed list/read operations into successful empty results. Partial unload/delete failures propagate to the caller. Failed metadata reads stop dependent label/group/user mutations. - Repaired calls to removed APIs in `del_shelve.py` and `delusers.py`, optional-instance argument indexing in `addusertogroup.py`, and `pymail.py` treating `-t` as an instance. Corrected label-list/instance interpretation in the label-conversion helper. - Rollback stops a file's work after failed sync/edit and refuses automatic submission of an incomplete rollback changelist. - Lowercase copy/rename utilities report failures and reject collisions rather than silently overwriting or reporting completion. - The file-list collector now loads the SDP environment and logs failed depot/directory/file queries, returns nonzero, and cleans up its temporary files on failure. Its existing selection policy is unchanged; see the inventory proposal below. - Deferred modern type annotations in nine P4Python utilities so imports work on the tested Python 3.9 runtime. This does not establish compatibility with the installer's much older Python default. Relevant implementation: [Maintenance helpers](../Maintenance/sdputils.py), [weekly maintenance](../Maintenance/maintenance), [rollback](../Maintenance/rollback.py), [unload clients](../Maintenance/unload_clients.py), [user deletion](../Maintenance/p4deleteuser.py). ### Setup, triggers, and Windows - `mkdirs.sh -test` now keeps its volume/directory and symlink operations in the preview tree instead of touching live DB/checkpoint locations. Setup diagnostics are retained. OS tuning no longer loops indefinitely when a setting already has the desired value. - Installer termination handlers retain failure/signal status instead of turning unsuccessful execution into success. Proxy cron and service templates use the selected instance and preserve argument boundaries; DTG loads its environment before using `P4TMP`. - Archive-transfer triggers preserve spaces/backslashes in filenames, use unique temporary files, and report failed file-list queries/transfers. - The case trigger detects new directories differing only in case within the same changelist. The label-options trigger restricts changes to its `Options:` field. Failed trigger metadata queries reject execution with diagnostics instead of proceeding with empty data. - Authentication fixes preserve colons in local passwords, continue to another RADIUS server after an exception, reject malformed OTP input, and remove an appended OTP before verifying the LDAP password. - Windows checkpoint handling preserves explicit parameters and configuration values, supports singleton journal lists under StrictMode, acquires an owned semaphore atomically, and does not delete another run's semaphore. It checks required login/counter operations and records success only after the checkpoint counter update succeeds. Relevant current implementation: [setup](../Server/Unix/setup/mkdirs.sh), [triggers](../Server/Unix/p4/common/bin/triggers). The Windows findings above concern the reviewed `daily_checkpoint.ps1` before its concurrent removal. ### Shared logging, snapshots, and downloads - Python structured logging promotes a nonzero result to failure even if a warning was recorded earlier. Prometheus error labels receive the same redaction as JSON output. Boolean/nonfinite values and duplicate normalized metric names no longer produce invalid/conflicting metrics. - Bash exit logging remains best effort under `set -e`: failure of its temporary diagnostics file does not change a successful operation's exit status. Prior opt-in text-log support remains intact. - AWS snapshot creation/tagging executes the AWS commands directly instead of attempting to execute their returned text as commands. Environment/login/counter checks and creation/tagging/listing/deletion errors are logged. Invalid dates cannot cause expiration. The existing 90-day retention and Contents-tag scope remain unchanged. - Binary/API downloads use temporary files and validate successful HTTP responses and content before replacement. Failed downloads preserve existing binaries/archives, retry state resets per binary, temporary files are cleaned up, and `-n` preserves existing binary permissions. Installed downloads have usable binary/archive permissions and a retained download log. Relevant implementation: [Python logger](../Server/Unix/p4/common/bin/sdp_structured_log.py), [Bash logger](../Server/Unix/p4/common/bin/sdp_structured_log.sh), [AWS snapshots](../Server/Unix/p4/common/cloud/aws/bin/snapshot.sh), [binary downloader](../p4_binaries/get_p4_binaries.sh). ## Approved improvements — pending changelist The approved second batch is implemented in the workspace and remains unsubmitted. The following list describes the current source behavior; the validation sections distinguish historical checks from reproducible Linux tests. 1. **Checkpoint ownership:** atomic creation with a host/process/token owner, acquired before shared log rotation across checkpoint/recovery/failover callers. Only the owner releases it after success. The shared `die()` failure handler also attempts an owner-checked release; it never removes another run's marker. Separate handlers such as `load_checkpoint.sh`'s `bail()` and interrupted runs can retain a marker for investigation; contention skips without changing the active owner's diagnostics. 2. **Recovery-chain retention:** complete generations count across checkpoint formats; journal retention includes the oldest retained checkpoint and offline replay requirements. Unknown state or gaps suppress pruning. `load_checkpoint.sh` and the shared offline-restore helper use a readable matching MD5 sidecar as the completion indicator, without a separate `p4d -jv`/`-jvp` prepass or checksum recomputation before replay. Actual replay failures remain errors. Edge recovery retains explicit checkpoint integrity verification. 3. **Recoverable database switches:** preflight, staged metadata and link changes, retained previous database, and explicit rollback on switch failures. Incomplete rollback leaves p4d stopped with a recovery directory. Edge recovery accepts validated serial or parallel checkpoints before replacing databases. 4. **Direct missing-file repair:** explicit detailed verify input (default `verify_output.log`), complete path validation before transfers, literal deduplication, and protected direct rsync arguments. No generated shell script. 5. **Maintenance isolation and fewer scans:** shared instance lock/private run directory, reusable validated client selection, one pending-notification scan, cached user lookups, and isolated standalone scratch/recovery files. The 200-client cap, selection age/order and Swarm exclusions remain. 6. **Bounded cleanup:** `SDP_CACHE_CLEAN_WORKERS` defaults to four (range 1–256). Cache/proxy cleanup logs per-depot counts and duration and waits for all workers. Cancellation stops/reaps find processes and preserves partial counts. 7. **Retained diagnostics and correlation:** archive previous run logs before replacement; defaults seven archives and 100 MiB per log, always preserving the newest archive. Run IDs connect text, JSON, verification CSV and Prometheus comments. All five detailed verification outputs are retained; a per-instance verification lock prevents concurrent runs from rotating active diagnostics. 8. **Edge scheduling:** the ordinary daily checkpoint skips the first Saturday; the recreation job retains its maintenance window. 9. **AWS and Azure backups:** AWS uses one coordinated multi-volume request, creation-time ownership tags, completion checks and scoped retention. Azure uses a dedicated owned restore-point collection and a crash-consistent VM restore point; all disk copies must complete before expiration. Unsupported Azure disk layouts fail without a per-disk fallback. See the [cloud guide](cloud_snapshots.md) for configuration and restore requirements. 10. **Trigger configuration and retirement:** configurable RADIUS endpoints, files/domain/binary and explicit Incident-trigger instance. At the user's subsequent request, the unused OTP trigger (`otpauthcheck.py`) and dedicated key generator (`otpkeygen.py`) are deleted; their proposed enhancements and dependencies are removed. RADIUS authentication policy remains unchanged. 11. **Runtime:** self-contained Python installer targeting CPython 3.14.8 and P4Python 2026.1, compatible wheel requirement, and numeric version comparisons. Actual distro/interpreter combinations exercised are recorded by the [container matrix](../test/containers/README.md). 12. **Disposable testing:** guarded exact root/account/instance/loopback endpoint, local source/binary inputs, meaningful command/content/checkpoint assertions, and isolated systemd containers. Test outputs distinguish mocked coverage from live server execution and external prerequisites. 13. **Complete inventory:** structured Perforce output includes depot-root files, spaces and deleted-only directories; publication is atomic and a failed query preserves the previous complete output. 14. **Protection parsing:** host-specific rows are handled while retained rows preserve their original syntax. Intentional `server.locks` removal remains unchanged in maintenance and recovery. See the [operational reference](operational_scripts.md), [Maintenance reference](maintenance_scripts.md), [authentication configuration](../Server/Unix/p4/common/bin/triggers/AUTHENTICATION.md), and [runtime guide](../Server/Unix/setup/RUNTIME_AND_TESTING.md) for usage. ## Documentation reconciliation The documentation audits cover the maintained Markdown/text files in the exported source inventory, including configuration/spec examples. The generated audit records the exact file and relative-link counts for the tested export. The main [Unix guide](SDP_Guide.Unix.md) now links to the current task-specific references. Historical findings below remain labeled as historical; removed Windows and OTP programs are not presented as available tools. | Area | Reconciliation | | --- | --- | | Main guide and Unix installation | Replaced obsolete duplicated instructions with configuration-file installation through `mkdirs.sh mkdirs.cfg`, actual paths/roles, initial-password ordering, service registration, generated cron review, backup initialization and deliberate migration/upgrade procedures. | | Operational scripts and p4verify reference | Removed the deleted cron verifier; corrected trigger phases, scoped label edits, failure statuses, inherited environment requirements, shared-standby identity, edge behavior, log retention, and the missing-archive-only verification scope. | | Recovery, edge, Cheat Sheet and best practices | Removed obsolete entry points and unsafe blanket journal/database deletion recipes; documented fencing, complete recovery sets, current helper boundaries, native edge failover prerequisites, and topology-specific rehearsal limits. | | Maintenance and server overview | Checked command arguments, current thresholds/report formats, helper APIs, logging, scheduling, and the distinction between client compatibility and supported server automation. | | Site and archive-trigger guides | Removed nonexistent automatic site hooks and corrected deployment/invocation guidance against the shipped implementation. | | Runtime, authentication, cloud and test guides | Checked dependencies, current interfaces, Azure/AWS ownership and restore boundaries, installer inputs and the distinction between live, mocked and reused test evidence. | | systemd, firewalld and network examples | Corrected service registration/control, unit/PID checks, firewall zone/runtime handling and conditional modern network configuration. | | Configuration/spec examples | Reviewed `Maintenance/change.txt`, `CheckCaseTrigger_FileFilter.txt` and `ssl/config.txt`; retained their format and site policy values. The installation guide explicitly requires replacing the SSL CN placeholder copied by `mkdirs.sh`. | Relative document links and referenced current entry points were checked against the maintained source inventory. Retired names remain only where explaining history or absence. This is a source/documentation reconciliation, not a claim that external authentication, cloud restoration, mail delivery or multi-host failover was exercised in the local containers. The seven obsolete Windows regression cases and PowerShell discovery were removed. The earlier two-platform Linux offline suites passed **337 tests with zero skips**. Their evidence records those runs separately from the earlier live tests, which are reused only after confirming unchanged production executable hashes. Documentation and test-retirement edits do not imply another live deployment or reinstallation. The installer manifest, current input manifest, and per-stage provenance are retained together. ## Expanded assurance follow-up The subsequent review found that overlapping standalone Maintenance runs could rotate and prune a log still held by another process. Maintenance logging now uses a short preparation mutex and a shared lifetime lease: overlapping operations append to the active log, and rotation occurs only when no active writer holds it. Real concurrent-process tests cover retention of one archive, default retention, simultaneous startup and termination. Client selection from a saved report also checks current server access immediately before each unload, preserving the cap and order while avoiding stale-report selection. Live 2026.1 topology setup exposed initial service passwords left expired by `dm.user.resetpassword`. The focused initialization helper honors that policy and performs the required self-change using stdin, without weakening the server setting or placing passwords in command arguments/files. Its failure paths and both policy settings have explicit regressions; the topology suite checks real replica authentication. Real proxy startup exposed a missing installer cache alias; `mkdirs.sh` now creates `/p4//cache` when absent while preserving existing paths and preview behavior. Stronger archive-loss tests also exposed untagged JSON depot discovery in `p4verify.py`: the verifier now requests typed depot fields explicitly and stops on unknown records. This enables its intended spec/unload checks as well as scoped verification. The live fixture then exposed spec archives written before `server.depot.root` was configured; setup now applies the configured archive root before creating those depots. Existing misplaced archives require diagnosis and recovery rather than being silently ignored. Edge recovery now stages its replacement beneath the writable physical offline database directory, allowing the normal layout with a root-owned parent. Both consecutive edge recoveries and a normal database switch between them are exercised. The downstream edge standby is explicitly reseeded after the rebuilt edge changes journal history, a requirement also present with the deployed archive; the recovery documentation now makes that step explicit. Standby promotion now reads and validates the first journal/offset record in `statejcopy`. A newer server's following source-server ID line previously became part of the journal filename and prevented promotion. Malformed or missing positions still stop before changing server identity; the legacy record without a final newline remains supported. Historical installation exposed configurable names unavailable in older p4d releases and an unconditional automation login with no automation users configured. The checker now logs documented release exclusions, refreshes only configured automation accounts, and continues to reject unknown settings and query failures. New-server configuration propagates checker/application failures instead of returning success after a warning. The expanded suite requires four OS user spaces, both interpreter variants, named paired baseline cases, real six-node recovery/failure scenarios and fourteen historical server-version/platform cells. It verifies source and image provenance and rejects missing cases, expected failures, unexpected skips and stale logs. Final run status comes from the generated acceptance report; the historical counts below do not establish that the expanded matrix passed. The [container guide](../test/containers/README.md) contains reproducible commands and the [production validation guide](production_validation.md) describes the remaining production rehearsal boundaries. ## Initial submission validation (historical) Before the concurrent Windows removal, the combined regression suite ran **141 tests: 140 passed, one skipped**. The skipped test requires two distinct filenames differing only in case, which this host filesystem cannot create. A separate mocked collision-guard test passed. After replacing the added shell options with explicit status checks, the suite completed successfully: **146 discovered, 138 passed, eight skipped** (the seven Windows cases plus the filesystem case above). Windows tests skip when their source is absent, as they already do when PowerShell is unavailable. The reviewed Windows implementation and its reconstruction inputs were preserved outside the workspace alongside the review artifacts. | Suite | Tests run | | --- | ---: | | Shared logging, snapshots, downloads, configuration startup | 16 | | Earlier maintenance logging fixes | 25 | | Maintenance review | 42 (one skipped) | | Existing limit recommendations | 13 | | Runtime and recovery review | 29 | | Setup, triggers, Windows review | 21 | ```sh SDP_TEST_BASH=/opt/homebrew/bin/bash \ SDP_TEST_PWSH=/tmp/sdp-pwsh/pwsh \ python3 -m unittest discover -s sdp/test -p 'test_*.py' -v ``` Seven of the setup/trigger/Windows cases executed with portable PowerShell 7.6.7 on macOS. They validate script logic with fixtures, not Windows service/filesystem integration. Bash tests used Homebrew Bash rather than macOS's older system Bash. Python tests used the host Python 3.9 runtime. Static checks passed for 75 shell files/templates, 70 Python files, and four JSON files, including active depot copies of locally absent files. All 39 opened shell files/templates passed ShellCheck at error severity. The initial submission's functional tests used temporary fixtures and mocked external commands/services. No real maintenance, recovery, authentication, cloud, email, or deployment actions were performed. In particular, neither the live test harness nor `reset.sh` was executed. After the requested rsync trust change, all **29 maintenance logging tests passed**, including four added regressions covering the transport inherited by all four remote-transfer scripts, explicit overrides, checkpoint/journal retries, and failed retries stopping before recovery. All four scripts passed Bash syntax checks and ShellCheck at error severity. An offline `ssh -G` check confirmed `accept-new` and the normal known-hosts paths; no remote SSH connections were made. ## Additional issues found during container validation - Historical clean installations exposed an empty depot-space response before `server.depot.root` was configured. The checker now applies an authorized archive-root fix before dependent disk-space checks, validates every numeric volume result and compares the least available space. Failed, empty, malformed and insufficient-space responses prevent the requested threshold change and contribute to failure status. Preview and `-fix`/`-FIX` scope remain explicit. - Service detection previously treated systemd's nonempty `not-found` output as an installed unit. p4d/broker/proxy start and stop now check `LoadState`, retain systemd control for disabled/masked units, and fall back only when no unit is available. Init-script failures are explicitly propagated. A focused regression exercises all six routes across absent, loaded, masked and query-failure cases. - Cache cancellation now stops and reaps actual deletion workers before cleanup and replaying the caller's signal handler. Partial counts remain in diagnostics. - Verification now takes a nonblocking per-instance kernel lock before rotating logs, preserving an active run's diagnostics when a manual/cron invocation overlaps. - Broker configuration generation now checks the broker-template and server configuration producers before rendering output; failures no longer become successful empty/default configuration files. - Directory ownership scanning propagates find/stat failures and preserves filenames with spaces, instead of reporting a clean scan after suppressed errors. - Replica cleanup and edge dumps coordinate with checkpoint ownership; cleanup reads the offline replay floor. Replica checkpoint requests report failed immediate journal rotation, and replica status rejects incomplete/malformed queries. Broker/proxy log rotation stops after failed authentication. - The RADIUS diagnostic now returns failure after denied/unreachable responses and tries later configured hosts after one connection fails. - Changelist updates close their input file on both success and query failure. - Run-log retention persists an increasing archive sequence in both languages. Identical filesystem timestamps or a clock correction can no longer select an older archive over the latest failure log; legacy archives remain supported. - Broker startup now preserves a failed daemon exit code. Broker/proxy stop timeouts return failure; restart does not start a second process after an unsuccessful stop. Cache purge stops after failed authentication. - Upgrade execution owns the checkpoint lock and stops on failed mutation or invalid version/configuration reads. Dry runs remain nonmutating. This is checked failure handling, not automatic schema-upgrade rollback. - Recent-change size reporting reads validated tagged changelist IDs and makes direct quoted commands, fixing human-readable output being treated as revision syntax. Known nonmatching server roles now skip successfully in role wrappers; unknown or missing role configuration still fails. ## Reproducible Linux validation The [container runner](../test/containers/README.md) installs the exported SDP through the real `mkdirs.sh` and a generated `mkdirs.cfg` derived from the shipped template. It records that configuration, its checksum, the installer command, full provisioning output, source hashes and exact P4/P4D versions. The current targets are Ubuntu 26.04 LTS and Red Hat UBI 8.10, 9.8 and 10.2 with systemd. UBI exercises RHEL userspace without requiring an entitled full RHEL installation. Each platform records offline regressions, a real single-server harness, additional runtime and Maintenance scenarios, an actual CPython/P4Python build, and a sudoers preview checked by `visudo`. Per-script `coverage.json` distinguishes live behavior, isolated scenarios and syntax-only checks. Refer to the saved result directory for the final outcomes tied to its exact source manifest. The separate six-node topology and fourteen historical release/platform cells are also required by the acceptance gate. Live cloud calls and external authentication services remain outside this local evidence; successful isolated tests do not certify those integrations.