From 81ace74db484db3fed6852e1fda7d460dc905773 Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 14:39:49 +0300 Subject: [PATCH 01/20] feat(provision): listen on loopback only Every client of the instance runs on the same Windows machine, and nothing on the operator network may reach the server. site now writes listen_addresses = 'localhost' with the other ALTER SYSTEM deltas; it is postmaster-context, so it lands in pending_restart like shared_buffers and the existing report covers it. The Linux CI job requires the value pending after the two site runs, since its service container is never restarted. --- .github/workflows/ci.yml | 16 ++ .../20261019-site-loopback-and-restart.md | 194 ++++++++++++++++++ internal/provision/config.go | 1 + 3 files changed, 211 insertions(+) create mode 100644 docs/plans/20261019-site-loopback-and-restart.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3b1bd4f..0088918 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -70,6 +70,22 @@ jobs: ./semibase site --database semiplot_dev --expected-major 17 ./semibase site --database semiplot_dev --expected-major 17 + # The service container is never restarted, so the loopback-only value + # stays pending; a pending value proves site wrote the delta. + - name: Require the listen_addresses delta + env: + PGHOST: localhost + PGUSER: postgres + PGPASSWORD: ci-super + run: | + set -euo pipefail + pending=$(psql -d postgres -tAc \ + "SELECT pending_restart FROM pg_settings WHERE name = 'listen_addresses'") + if [[ "$pending" != t ]]; then + echo "::error::listen_addresses pending_restart is '$pending' (want t)" + exit 1 + fi + # Every other step runs semiplot_register_new_pens() over an empty # public.trends, so its INSERT adds nothing. - name: Register pens and probe the semiplot_tags constraints diff --git a/docs/plans/20261019-site-loopback-and-restart.md b/docs/plans/20261019-site-loopback-and-restart.md new file mode 100644 index 0000000..6c53ac0 --- /dev/null +++ b/docs/plans/20261019-site-loopback-and-restart.md @@ -0,0 +1,194 @@ +# Site: loopback-only listening and an optional service restart + +## Overview +- `semibase site` sets `listen_addresses = 'localhost'`: every client of the instance runs on the + same Windows machine, and nothing on the operator network may reach the server. +- `semibase site --restart-service ` restarts that Windows service when, and only when, the + tuning phase leaves settings waiting for a restart, then waits for the server to accept + connections and continues. Without the flag the run behaves as today and warns. +- The Windows installer (`20261019-windows-installer.md`) is the caller of the flag. CI's Linux job + runs `site` against a service container the tool cannot restart, so the restart stays opt-in. + +## Context (from discovery) +- `internal/provision/config.go:16-28` — the `settings` list applied with `ALTER SYSTEM`; + `config.go:30-71` applies it, reloads, and reports `pending_restart` (`config.go:60-70`); + `config.go:36-40` opens the one connection the phase uses. +- `internal/provision/provision.go:39-44` — `Options.Validate`; `provision.go:48-56` — `Site` runs + `config`, `create`, `check` in that order; `provision.go:26-35` — `Options`. +- `internal/provision/check.go:422` — `reportPendingRestart` at the tail of every run. +- `cmd/semibase/main.go:158-175` — one flag set shared by `site` and `bench`; the comment at + `main.go:154-155` states every flag is used by either command; `main.go:184` calls `Validate`. +- `cmd/semibase/main.go:60,112` — one five-minute timeout covers the whole command. +- `go.mod:5` — the only direct dependency is `pgx/v5`. +- `.github/workflows/ci.yml:11-29` — the Windows job builds, tests and lints only; + `ci.yml:31-47` — the Linux job's service container and its password `ci-super`; + `ci.yml:65-71` — the Linux job runs `site` twice against `postgres:17-alpine`. +- `docs/architecture/provisioning.md:160-165` — "The tool never touches the service manager." +- `docs/architecture/configuration.md:57-58` — "listens on the loopback interface plus the operator + network only"; this no longer holds. +- Read 2026-10-19 from `actions/runner-images` `images/windows/Windows2025-Readme.md` + (`windows-latest`): PostgreSQL 17.11, service `postgresql-x64-17`, status Stopped, start type + Disabled, user `postgres`, password `root`, `PGBIN=C:\Program Files\PostgreSQL\17\bin`, + `PGDATA=C:\PostgreSQL\17\data`. + +## Development Approach +- **testing approach**: Regular (code first, then tests in the same task) +- complete each task fully before moving to the next +- every task includes new or updated tests for the code it changes +- all tests pass before the next task starts +- update this plan when scope changes + +## Testing Strategy +- unit tests beside the source, table-driven (`cmd/semibase/main_test.go`, + `internal/provision/*_test.go`); platform-specific halves live in build-tagged test files +- integration: CI steps against live servers; the repository has no database-touching unit tests + (CLAUDE.md, Test section) + +## Acceptance Evidence +- Linux CI, after the two `site` runs (`ci.yml:70-71`): + `PGHOST=localhost PGUSER=postgres PGPASSWORD=ci-super psql -d postgres -tAc "SELECT pending_restart FROM pg_settings WHERE name = 'listen_addresses'"` + prints `t`. The container is never restarted, so the value stays pending; this proves the delta + is written. +- Windows CI, new step against the runner's PostgreSQL 17, with `$psql = "$env:PGBIN\psql.exe"`: + 1. `semibase site --port 5432 --expected-major 17 --restart-service postgresql-x64-17` exits 0, + and its output contains `restarting service postgresql-x64-17`. + 2. `& $psql -h 127.0.0.1 -U postgres -d postgres -tAc "SHOW listen_addresses"` prints + `localhost`, and `SELECT count(*) FROM pg_settings WHERE pending_restart` prints `0`. + 3. A second identical run exits 0, and its output does not contain `restarting service`. +- Unit tests: `bench --restart-service x` is a usage error; `--restart-service` with a + non-loopback `--host` is a usage error; `site --restart-service x` with a loopback host parses. + +## Solution Overview +- `listen_addresses` joins the `settings` list. It is postmaster-context, so it lands in + `pending_restart` exactly like `shared_buffers`, and the existing reporting covers it. +- The restart happens inside the tuning phase, right after `pendingRestartSettings` returns a + non-empty list, so `create` and `check` run against the restarted server and the tail's + `reportPendingRestart` reads an empty list. +- Service control uses `golang.org/x/sys/windows/svc/mgr` behind a `windows` build tag, not + `net stop`/`net start`: its errors do not depend on the Windows display language, it reads the + service's start type, so a Disabled service is refused before any change, and it exposes the + service state for bounded waits. `x/sys` is pure Go, so cross-compilation without cgo stays. +- The flag parses on every platform. The service is opened and checked at the start of + `config`, before it connects: a wrong name, a missing right, a Disabled start type, or a + non-Windows build (`service_other.go` returns "service restart is not supported on ") + fails with nothing changed. The module stays one cross-platform build: the Linux binary is what + the docker benches run. +- Once Stop has been sent, Start is always sent, on a context detached from the command's + cancellation and timeout (`context.WithoutCancel`) with its own bound. An interrupt or a timeout + during the restart never leaves the service stopped. +- After the service reports Running, the tool reconnects in a bounded loop. Whether an EDB + service reports Running before the postmaster accepts connections is unverified; the loop costs + one successful connect when it does not. + +## Technical Details +- `Options.RestartService string`; empty means no restart. +- `--restart-service `, accepted by `site` only. `Validate` refuses it when the command + is `bench` (no tuning, so nothing it does creates a pending restart) and when `--host` is not + `localhost`, `127.0.0.1` or `::1`: after the restart the server listens on loopback only. +- Decision function `restartNeeded(pending []string, service string) bool`: true only when both + are non-empty. +- Restart sequence in `config`: + 1. Close the phase connection. + 2. `Stop`, then wait for `Stopped`, bounded by 3 minutes: a shutdown checkpoint with + `max_wal_size = 8GB` can be long. + 3. `Start` on `context.WithoutCancel(ctx)`, then wait for `Running`, bounded by 2 minutes. This + step runs whether or not the stop wait succeeded. + 4. The ready wait connects to `postgres` as the superuser every second, bounded by 60 s. It + returns the new connection, and the last connect error goes into the timeout message. + 5. `pendingRestartSettings` runs on the new connection. A non-empty list is an error naming the + settings. +- Errors name the service and its last observed state. +- Output: `step("restarting service %s: %s waiting for a restart", name, list)`, then `ok` lines + for stopped, running, accepting connections. + +## What Goes Where +- Implementation Steps: code, tests, CI steps and docs in this repository. +- Post-Completion: checks on an installation machine. + +## Implementation Steps + +### Task 1: Listen on loopback only + +**Files:** +- Modify: `internal/provision/config.go` +- Modify: `.github/workflows/ci.yml` + +- [x] add `{"listen_addresses", "localhost"}` to `settings` (`config.go:16-28`) +- [x] add the Linux CI check from Acceptance Evidence after `ci.yml:71`; it is this task's test, + since a unit test of a constant list proves nothing +- [x] run `go test ./...` and `go vet ./...` - must pass before task 2 + +### Task 2: Parse and validate `--restart-service` + +**Files:** +- Modify: `internal/provision/provision.go` +- Modify: `cmd/semibase/main.go` +- Modify: `cmd/semibase/main_test.go` + +- [ ] add `RestartService` and the command name `Validate` needs to `Options` + (`provision.go:26-35`) +- [ ] extend `Validate` (`provision.go:39-44`) with the two refusals from Technical Details +- [ ] register `--restart-service` in `newFlagSet` (`main.go:158`); rewrite the comment at + `main.go:154-155`, which stops being true; add the flag to `usage` (`main.go:20-55`) +- [ ] write table tests: `bench` rejects the flag, a non-loopback host rejects it, `site` with a + loopback host accepts it, no flag parses as today +- [ ] run `go test ./...` - must pass before task 3 + +### Task 3: Restart the service and wait for the server + +**Files:** +- Create: `internal/provision/service_windows.go`, `internal/provision/service_other.go` +- Modify: `internal/provision/config.go` +- Create: `internal/provision/config_test.go` +- Modify: `go.mod`, `go.sum` + +- [ ] add `golang.org/x/sys`; in `service_windows.go` implement opening and checking the service + (exists, access, not Disabled) and the stop-start sequence with the bounds above +- [ ] `service_other.go` returns an error naming the platform from the same check function +- [ ] in `config`: check the service before connecting; after + `pendingRestartSettings` (`config.go:60`) call `restartNeeded`, run the sequence, re-read + on the new connection +- [ ] extract the ready wait over an injected `func(ctx) (*pgx.Conn, error)` dialer +- [ ] write tests: `restartNeeded` table; ready wait succeeds at once, succeeds after N failures, + times out with the last error, stops on context cancellation +- [ ] run `go test ./...`, `go vet ./...`, `golangci-lint run`; `GOOS=linux go build ./...` - + must pass before task 4 + +### Task 4: Exercise the restart on the Windows runner + +**Files:** +- Modify: `.github/workflows/ci.yml` + +- [ ] add a step to the Windows job: `Set-Service postgresql-x64-17 -StartupType Manual`, + `Start-Service postgresql-x64-17` +- [ ] run `semibase site` with `SEMIBASE_SUPER_PASSWORD=root`, writer and plot passwords set + (the first run creates those roles), and the flags from Acceptance Evidence +- [ ] run the three Windows checks from Acceptance Evidence; this step is the only test of the + service sequence itself +- [ ] push the branch and read the CI run - both jobs green before task 5 + +### Task 5: Verify acceptance criteria +- [ ] every item in Acceptance Evidence holds on the CI run +- [ ] `go test -race ./...`, `go vet ./...`, `golangci-lint run`, `gofmt -l .` prints nothing + +### Task 6: [Final] Update documentation +- [ ] `docs/architecture/provisioning.md:160-165`: the opt-in restart, where it runs, the + Start guarantee, why CI's Linux job does not use it; `provisioning.md:94` follows +- [ ] `docs/architecture/configuration.md`: `listen_addresses` in the deltas table; + `configuration.md:15-16` and `:29` stop calling `shared_buffers` the only restart setting; + `configuration.md:57-58` becomes loopback only, single machine, and notes that + `postgresql.auto.conf` overrides `postgresql.conf` +- [ ] `docs/architecture/README.md:22` and `docs/architecture/overview.md:61` (provisioning + step 3) name the flag +- [ ] `docs/deployment.md`, in Russian: the flags table gets `--restart-service`; step 4 says the + flag does the restart; one sentence that a restart drops the SCADA's connections, so the + SCADA is stopped before a re-run on a working site +- [ ] "memory constants" and "memory tuning" become "server settings" in `CLAUDE.md`, + `main.go:26`; "pure `pgx`" becomes "pure Go" in `CLAUDE.md`, `release.yml:124` and + `Dockerfile:3-4`; the Run section of `CLAUDE.md` mentions the flag +- [ ] move this plan to `docs/plans/completed/` + +## Post-Completion +- On an installation machine, confirm the Simple-Scada project connects to `localhost` or + `127.0.0.1`. A project that names the machine's own IP or host name stops writing after the + restart. diff --git a/internal/provision/config.go b/internal/provision/config.go index 0bebb71..c7fb17e 100644 --- a/internal/provision/config.go +++ b/internal/provision/config.go @@ -25,6 +25,7 @@ var settings = []setting{ {"random_page_cost", "1.1"}, {"log_min_duration_statement", "1000"}, {"track_io_timing", "on"}, + {"listen_addresses", "localhost"}, } func (o Options) config(ctx context.Context) error { From ae435d361c2feb66582ed9f0e7bb9bf465feb08f Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 14:43:54 +0300 Subject: [PATCH 02/20] feat(cmd): accept --restart-service for site The installer needs site to restart the PostgreSQL service when the tuning leaves settings waiting for a restart. The flag parses on every platform; Options.Validate refuses it for bench, which applies no server settings, and for a host other than localhost, 127.0.0.1 or ::1, since the restarted server listens on loopback only. Options carries the command name so Validate can tell site from bench. The restart itself lands in the next commit; until then the flag is validated and ignored. --- cmd/semibase/main.go | 14 +++-- cmd/semibase/main_test.go | 52 ++++++++++++++++++- .../20261019-site-loopback-and-restart.md | 10 ++-- internal/provision/provision.go | 17 ++++++ 4 files changed, 84 insertions(+), 9 deletions(-) diff --git a/cmd/semibase/main.go b/cmd/semibase/main.go index f5b3bb2..c409499 100644 --- a/cmd/semibase/main.go +++ b/cmd/semibase/main.go @@ -33,6 +33,11 @@ refused an insert, a delete or a key change on semiplot_tags; it writes neither archive nor semiplot_meta and holds no CREATE on schema public. A failed check is a non-zero exit. +The tuning includes settings that take effect only after a server restart. site +reports them and leaves the restart to the operator, unless --restart-service names +the Windows service to restart; the restart needs a loopback --host, because the +server then listens on the loopback interface only. + Every step checks before it creates; re-running either command is safe. Passwords of existing roles change only when the corresponding flag or variable is set. @@ -146,8 +151,8 @@ func revisionFromSettings(settings []debug.BuildSetting) string { return vcsRevision } -// One flag set serves both commands: site and bench differ only in the tuning phase, -// which reads no flag of its own, so every flag below is used by either command. +// One flag set serves both commands, which differ only in the tuning phase. The tuning +// phase's one flag, --restart-service, is refused for bench by Options.Validate. // Password flags default to empty so the usage output can only ever show the variable // names, never their values. func newFlagSet(command string, options *provision.Options) *flag.FlagSet { @@ -166,11 +171,14 @@ func newFlagSet(command string, options *provision.Options) *flag.FlagSet { "semiplot password (env SEMIBASE_PLOT_PASSWORD)") flags.IntVar(&options.ExpectedMajor, "expected-major", 0, "required server major version; 0 accepts any major >= 14") + flags.StringVar(&options.RestartService, "restart-service", "", + "site only: Windows service to restart when the server settings wait for a restart; "+ + "needs a loopback --host") return flags } func parseOptions(command string, arguments []string) (provision.Options, error) { - options := provision.Options{} + options := provision.Options{Command: command} flags := newFlagSet(command, &options) if err := flags.Parse(arguments); err != nil { return provision.Options{}, err diff --git a/cmd/semibase/main_test.go b/cmd/semibase/main_test.go index 9ce3346..0ca4005 100644 --- a/cmd/semibase/main_test.go +++ b/cmd/semibase/main_test.go @@ -85,12 +85,58 @@ func TestFlagUsageHidesPasswordValues(t *testing.T) { func TestParseOptions(t *testing.T) { tests := []struct { name string + command string arguments []string environment map[string]string wantErr error wantAnyErr bool check func(t *testing.T, options provision.Options) }{ + { + name: "no restart flag parses as before", + check: func(t *testing.T, options provision.Options) { + if options.RestartService != "" { + t.Errorf("RestartService = %q, want empty", options.RestartService) + } + }, + }, + { + name: "bench rejects --restart-service", + arguments: []string{"--restart-service", "x"}, + wantAnyErr: true, + }, + { + name: "site rejects --restart-service with a non-loopback host", + command: "site", + arguments: []string{"--restart-service", "x", "--host", "db.internal"}, + wantAnyErr: true, + }, + { + name: "site rejects --restart-service with a socket host", + command: "site", + arguments: []string{"--restart-service", "x", "--host", "/var/run/postgresql"}, + wantAnyErr: true, + }, + { + name: "site accepts --restart-service with the default host", + command: "site", + arguments: []string{"--restart-service", "postgresql-x64-17"}, + check: func(t *testing.T, options provision.Options) { + if options.RestartService != "postgresql-x64-17" { + t.Errorf("RestartService = %q, want %q", options.RestartService, "postgresql-x64-17") + } + }, + }, + { + name: "site accepts --restart-service with 127.0.0.1", + command: "site", + arguments: []string{"--restart-service", "x", "--host", "127.0.0.1"}, + }, + { + name: "site accepts --restart-service with ::1", + command: "site", + arguments: []string{"--restart-service", "x", "--host", "::1"}, + }, { name: "flag beats environment", arguments: []string{"--super-password", "from-flag"}, @@ -134,7 +180,11 @@ func TestParseOptions(t *testing.T) { t.Setenv(name, value) } - options, err := parseOptions("bench", tt.arguments) + command := tt.command + if command == "" { + command = "bench" + } + options, err := parseOptions(command, tt.arguments) switch { case tt.wantErr != nil: diff --git a/docs/plans/20261019-site-loopback-and-restart.md b/docs/plans/20261019-site-loopback-and-restart.md index 6c53ac0..c1e9518 100644 --- a/docs/plans/20261019-site-loopback-and-restart.md +++ b/docs/plans/20261019-site-loopback-and-restart.md @@ -125,14 +125,14 @@ - Modify: `cmd/semibase/main.go` - Modify: `cmd/semibase/main_test.go` -- [ ] add `RestartService` and the command name `Validate` needs to `Options` +- [x] add `RestartService` and the command name `Validate` needs to `Options` (`provision.go:26-35`) -- [ ] extend `Validate` (`provision.go:39-44`) with the two refusals from Technical Details -- [ ] register `--restart-service` in `newFlagSet` (`main.go:158`); rewrite the comment at +- [x] extend `Validate` (`provision.go:39-44`) with the two refusals from Technical Details +- [x] register `--restart-service` in `newFlagSet` (`main.go:158`); rewrite the comment at `main.go:154-155`, which stops being true; add the flag to `usage` (`main.go:20-55`) -- [ ] write table tests: `bench` rejects the flag, a non-loopback host rejects it, `site` with a +- [x] write table tests: `bench` rejects the flag, a non-loopback host rejects it, `site` with a loopback host accepts it, no flag parses as today -- [ ] run `go test ./...` - must pass before task 3 +- [x] run `go test ./...` - must pass before task 3 ### Task 3: Restart the service and wait for the server diff --git a/internal/provision/provision.go b/internal/provision/provision.go index 7874102..966ff1e 100644 --- a/internal/provision/provision.go +++ b/internal/provision/provision.go @@ -7,6 +7,7 @@ import ( "net" "net/url" "regexp" + "slices" "strconv" "strings" @@ -26,6 +27,7 @@ const versionFloor = 140000 const archiveTable = "public.trends" type Options struct { + Command string Host string Port int Database string @@ -34,14 +36,29 @@ type Options struct { WriterPassword string PlotPassword string ExpectedMajor int + RestartService string } var databaseNamePattern = regexp.MustCompile(`^[a-z_][a-z0-9_]*$`) +var loopbackHosts = []string{"localhost", "127.0.0.1", "::1"} + func (o Options) Validate() error { if !databaseNamePattern.MatchString(o.Database) { return fmt.Errorf("database name %q must match %s", o.Database, databaseNamePattern) } + if o.RestartService == "" { + return nil + } + if o.Command != "site" { + return fmt.Errorf("--restart-service is accepted by site only: %s applies no server "+ + "settings, so nothing it does waits for a restart", o.Command) + } + if !slices.Contains(loopbackHosts, o.Host) { + return fmt.Errorf("--restart-service needs --host to be one of %s, not %q: after the "+ + "restart the server listens on the loopback interface only", + strings.Join(loopbackHosts, ", "), o.Host) + } return nil } From 30373a7ed450a392e3b16c4bb033dadd5e6a90df Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 14:53:32 +0300 Subject: [PATCH 03/20] feat(provision): restart the service on demand The tuning phase leaves shared_buffers and listen_addresses waiting for a restart, so create and check ran against the old settings and the run ended with a warning. With --restart-service the phase now restarts the named Windows service through x/sys svc/mgr, waits for the server to accept connections again, and re-reads pending_restart. The service is checked before the first connection: a missing or Disabled service, a stopped one, a missing right, or a non-Windows build fails with nothing changed. Once Stop is accepted, Start is sent on a context detached from the command's cancellation, so an interrupt or a timeout never leaves the service stopped. --- .../20261019-site-loopback-and-restart.md | 12 +- go.mod | 5 +- go.sum | 2 + internal/provision/config.go | 98 +++++++++-- internal/provision/config_test.go | 103 ++++++++++++ internal/provision/service_other.go | 22 +++ internal/provision/service_other_test.go | 20 +++ internal/provision/service_windows.go | 153 ++++++++++++++++++ 8 files changed, 398 insertions(+), 17 deletions(-) create mode 100644 internal/provision/config_test.go create mode 100644 internal/provision/service_other.go create mode 100644 internal/provision/service_other_test.go create mode 100644 internal/provision/service_windows.go diff --git a/docs/plans/20261019-site-loopback-and-restart.md b/docs/plans/20261019-site-loopback-and-restart.md index c1e9518..70119f1 100644 --- a/docs/plans/20261019-site-loopback-and-restart.md +++ b/docs/plans/20261019-site-loopback-and-restart.md @@ -142,16 +142,16 @@ - Create: `internal/provision/config_test.go` - Modify: `go.mod`, `go.sum` -- [ ] add `golang.org/x/sys`; in `service_windows.go` implement opening and checking the service +- [x] add `golang.org/x/sys`; in `service_windows.go` implement opening and checking the service (exists, access, not Disabled) and the stop-start sequence with the bounds above -- [ ] `service_other.go` returns an error naming the platform from the same check function -- [ ] in `config`: check the service before connecting; after +- [x] `service_other.go` returns an error naming the platform from the same check function +- [x] in `config`: check the service before connecting; after `pendingRestartSettings` (`config.go:60`) call `restartNeeded`, run the sequence, re-read on the new connection -- [ ] extract the ready wait over an injected `func(ctx) (*pgx.Conn, error)` dialer -- [ ] write tests: `restartNeeded` table; ready wait succeeds at once, succeeds after N failures, +- [x] extract the ready wait over an injected `func(ctx) (*pgx.Conn, error)` dialer +- [x] write tests: `restartNeeded` table; ready wait succeeds at once, succeeds after N failures, times out with the last error, stops on context cancellation -- [ ] run `go test ./...`, `go vet ./...`, `golangci-lint run`; `GOOS=linux go build ./...` - +- [x] run `go test ./...`, `go vet ./...`, `golangci-lint run`; `GOOS=linux go build ./...` - must pass before task 4 ### Task 4: Exercise the restart on the Windows runner diff --git a/go.mod b/go.mod index 93865b2..edff430 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,10 @@ module github.com/Semiteq/SemiBase go 1.26 -require github.com/jackc/pgx/v5 v5.10.0 +require ( + github.com/jackc/pgx/v5 v5.10.0 + golang.org/x/sys v0.47.0 +) require ( github.com/jackc/pgpassfile v1.0.0 // indirect diff --git a/go.sum b/go.sum index c0e505b..c48822d 100644 --- a/go.sum +++ b/go.sum @@ -18,6 +18,8 @@ github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= golang.org/x/sync v0.17.0 h1:l60nONMj9l5drqw6jlhIELNv9I0A4OFgRsG9k2oT9Ug= golang.org/x/sync v0.17.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/text v0.29.0 h1:1neNs90w9YzJ9BocxfsQNHKuAT4pkghyXc4nhZ6sJvk= golang.org/x/text v0.29.0/go.mod h1:7MhJOA9CD2qZyOKYazxdYMF85OwPdEr9jTtBpO7ydH4= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/internal/provision/config.go b/internal/provision/config.go index c7fb17e..e184a9d 100644 --- a/internal/provision/config.go +++ b/internal/provision/config.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "strings" + "time" "github.com/jackc/pgx/v5" ) @@ -28,49 +29,126 @@ var settings = []setting{ {"listen_addresses", "localhost"}, } +const ( + readyTimeout = 60 * time.Second + readyInterval = time.Second +) + func (o Options) config(ctx context.Context) error { if err := o.Validate(); err != nil { return err } step("Phase config: ALTER SYSTEM deltas") - conn, err := o.connectSuperuser(ctx, "postgres") + var service *managedService + if o.RestartService != "" { + var err error + // SA4023: off Windows openService always fails, by design + if service, err = openService(o.RestartService); err != nil { //nolint:staticcheck + return err + } + defer service.close() + } + + pending, err := o.applySettings(ctx) if err != nil { return err } + switch { + case restartNeeded(pending, o.RestartService): + return o.restartService(ctx, service, pending) + case len(pending) > 0: + note("waiting for a service restart: %s. Restart the PostgreSQL service or reboot the machine.", + strings.Join(pending, ", ")) + default: + ok("all settings active") + } + return nil +} + +// applySettings closes its connection before returning: a restart must not find it open. +func (o Options) applySettings(ctx context.Context) ([]string, error) { + conn, err := o.connectSuperuser(ctx, "postgres") + if err != nil { + return nil, err + } defer conn.Close(ctx) for _, s := range settings { statement := fmt.Sprintf("ALTER SYSTEM SET %s = '%s'", s.Name, s.Value) if _, err := conn.Exec(ctx, statement); err != nil { - return fmt.Errorf("%s: %w", statement, err) + return nil, fmt.Errorf("%s: %w", statement, err) } ok("ALTER SYSTEM SET %s = '%s'", s.Name, s.Value) } if _, err := conn.Exec(ctx, "SELECT pg_reload_conf()"); err != nil { - return fmt.Errorf("reloading the configuration: %w", err) + return nil, fmt.Errorf("reloading the configuration: %w", err) } // pg_reload_conf signals asynchronously; the pause lets the server processes apply // the new files before pending_restart is read if _, err := conn.Exec(ctx, "SELECT pg_sleep(0.5)"); err != nil { - return fmt.Errorf("waiting for the reload: %w", err) + return nil, fmt.Errorf("waiting for the reload: %w", err) } ok("configuration reloaded") - pending, err := pendingRestartSettings(ctx, conn) + return pendingRestartSettings(ctx, conn) +} + +func restartNeeded(pending []string, service string) bool { + return len(pending) > 0 && service != "" +} + +func (o Options) restartService(ctx context.Context, service *managedService, pending []string) error { + step("restarting service %s: %s waiting for a restart", o.RestartService, strings.Join(pending, ", ")) + if err := service.restart(ctx); err != nil { + return err + } + conn, err := waitReady(ctx, readyTimeout, readyInterval, func(ctx context.Context) (*pgx.Conn, error) { + return o.connectSuperuser(ctx, "postgres") + }) if err != nil { return err } - if len(pending) == 0 { - ok("all settings active") - return nil + defer conn.Close(ctx) + ok("server accepting connections") + + stillPending, err := pendingRestartSettings(ctx, conn) + if err != nil { + return err + } + if len(stillPending) > 0 { + return fmt.Errorf("after restarting service %s, settings still wait for a restart: %s", + o.RestartService, strings.Join(stillPending, ", ")) } - note("waiting for a service restart: %s. Restart the PostgreSQL service or reboot the machine.", - strings.Join(pending, ", ")) + ok("all settings active") return nil } +// waitReady dials at once, then every interval, until a dial succeeds or timeout elapses. +func waitReady( + ctx context.Context, + timeout, interval time.Duration, + dial func(context.Context) (*pgx.Conn, error), +) (*pgx.Conn, error) { + waitCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + for { + conn, err := dial(waitCtx) + if err == nil { + return conn, nil + } + select { + case <-waitCtx.Done(): + if ctx.Err() != nil { + return nil, fmt.Errorf("waiting for the server to accept connections: %w", ctx.Err()) + } + return nil, fmt.Errorf("the server accepted no connection within %s: %w", timeout, err) + case <-time.After(interval): + } + } +} + func pendingRestartSettings(ctx context.Context, conn *pgx.Conn) ([]string, error) { rows, err := conn.Query(ctx, "SELECT name FROM pg_settings WHERE pending_restart ORDER BY name") if err != nil { diff --git a/internal/provision/config_test.go b/internal/provision/config_test.go new file mode 100644 index 0000000..7773afa --- /dev/null +++ b/internal/provision/config_test.go @@ -0,0 +1,103 @@ +package provision + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/jackc/pgx/v5" +) + +func TestRestartNeeded(t *testing.T) { + tests := []struct { + name string + pending []string + service string + want bool + }{ + {"nothing pending, no service", nil, "", false}, + {"nothing pending, service named", nil, "postgresql-x64-17", false}, + {"pending, no service", []string{"shared_buffers"}, "", false}, + {"pending, service named", []string{"listen_addresses", "shared_buffers"}, "postgresql-x64-17", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := restartNeeded(tt.pending, tt.service); got != tt.want { + t.Errorf("restartNeeded(%v, %q) = %v, want %v", tt.pending, tt.service, got, tt.want) + } + }) + } +} + +var errRefused = errors.New("connection refused") + +// failingDialer returns a dialer that fails failures times, then returns conn, and its call count. +func failingDialer(failures int, conn *pgx.Conn) (func(context.Context) (*pgx.Conn, error), *int) { + calls := 0 + return func(context.Context) (*pgx.Conn, error) { + calls++ + if calls <= failures { + return nil, errRefused + } + return conn, nil + }, &calls +} + +func TestWaitReady(t *testing.T) { + conn := &pgx.Conn{} + tests := []struct { + name string + failures int + timeout time.Duration + wantCalls int + wantErr bool + }{ + {"succeeds at once", 0, time.Second, 1, false}, + {"succeeds after three failures", 3, time.Second, 4, false}, + {"times out with the last error", 1 << 30, 50 * time.Millisecond, 0, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dial, calls := failingDialer(tt.failures, conn) + got, err := waitReady(context.Background(), tt.timeout, time.Millisecond, dial) + if tt.wantErr { + if !errors.Is(err, errRefused) { + t.Fatalf("waitReady() error = %v, want it to wrap %v", err, errRefused) + } + if got != nil { + t.Errorf("waitReady() returned a connection with error %v", err) + } + return + } + if err != nil { + t.Fatalf("waitReady() error = %v", err) + } + if got != conn { + t.Errorf("waitReady() returned %p, want the dialed connection %p", got, conn) + } + if *calls != tt.wantCalls { + t.Errorf("dial called %d times, want %d", *calls, tt.wantCalls) + } + }) + } +} + +func TestWaitReadyStopsOnCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + calls := 0 + dial := func(context.Context) (*pgx.Conn, error) { + calls++ + if calls == 2 { + cancel() + } + return nil, errRefused + } + _, err := waitReady(ctx, time.Minute, time.Millisecond, dial) + if !errors.Is(err, context.Canceled) { + t.Fatalf("waitReady() error = %v, want it to wrap %v", err, context.Canceled) + } + if calls != 2 { + t.Errorf("dial called %d times after the cancellation, want 2", calls) + } +} diff --git a/internal/provision/service_other.go b/internal/provision/service_other.go new file mode 100644 index 0000000..22afdd7 --- /dev/null +++ b/internal/provision/service_other.go @@ -0,0 +1,22 @@ +//go:build !windows + +package provision + +import ( + "context" + "errors" + "fmt" + "runtime" +) + +type managedService struct{} + +func openService(name string) (*managedService, error) { + return nil, fmt.Errorf("--restart-service %s: service restart is not supported on %s", name, runtime.GOOS) +} + +func (*managedService) close() {} + +func (*managedService) restart(context.Context) error { + return errors.New("service restart is not supported on " + runtime.GOOS) +} diff --git a/internal/provision/service_other_test.go b/internal/provision/service_other_test.go new file mode 100644 index 0000000..f743c76 --- /dev/null +++ b/internal/provision/service_other_test.go @@ -0,0 +1,20 @@ +//go:build !windows + +package provision + +import ( + "runtime" + "strings" + "testing" +) + +func TestOpenServiceIsRefusedOffWindows(t *testing.T) { + service, err := openService("postgresql-x64-17") + if err == nil { + service.close() + t.Fatal("openService() succeeded off Windows") + } + if want := "not supported on " + runtime.GOOS; !strings.Contains(err.Error(), want) { + t.Errorf("openService() error = %q, want it to contain %q", err, want) + } +} diff --git a/internal/provision/service_windows.go b/internal/provision/service_windows.go new file mode 100644 index 0000000..cd0d349 --- /dev/null +++ b/internal/provision/service_windows.go @@ -0,0 +1,153 @@ +//go:build windows + +package provision + +import ( + "context" + "errors" + "fmt" + "time" + + "golang.org/x/sys/windows" + "golang.org/x/sys/windows/svc" + "golang.org/x/sys/windows/svc/mgr" +) + +const ( + // a shutdown checkpoint with max_wal_size = 8GB can be long + stopTimeout = 3 * time.Minute + startTimeout = 2 * time.Minute + statePollInterval = 500 * time.Millisecond +) + +type managedService struct { + name string + manager *mgr.Mgr + handle *mgr.Service +} + +// openService refuses a service the restart could not complete: missing, out of reach, +// Disabled, or not running, which means it is not the server the run connects to. +func openService(name string) (*managedService, error) { + manager, err := mgr.Connect() + if err != nil { + if errors.Is(err, windows.ERROR_ACCESS_DENIED) { + return nil, fmt.Errorf("service %s: access to the service control manager denied; "+ + "run semibase as an administrator", name) + } + return nil, fmt.Errorf("service %s: connecting to the service control manager: %w", name, err) + } + handle, err := manager.OpenService(name) + if err != nil { + _ = manager.Disconnect() + switch { + case errors.Is(err, windows.ERROR_SERVICE_DOES_NOT_EXIST): + return nil, fmt.Errorf("service %s does not exist", name) + case errors.Is(err, windows.ERROR_ACCESS_DENIED): + return nil, fmt.Errorf("service %s: access denied; run semibase as an administrator", name) + } + return nil, fmt.Errorf("opening service %s: %w", name, err) + } + service := &managedService{name: name, manager: manager, handle: handle} + if err := service.checkRestartable(); err != nil { + service.close() + return nil, err + } + return service, nil +} + +func (s *managedService) checkRestartable() error { + config, err := s.handle.Config() + if err != nil { + return fmt.Errorf("reading the configuration of service %s: %w", s.name, err) + } + if config.StartType == mgr.StartDisabled { + return fmt.Errorf("service %s is Disabled, so it could not be started again after the stop; "+ + "set its start type to Automatic or Manual", s.name) + } + status, err := s.handle.Query() + if err != nil { + return fmt.Errorf("querying service %s: %w", s.name, err) + } + if status.State != svc.Running { + return fmt.Errorf("service %s is %s, not running, so it is not the server this run "+ + "connects to", s.name, stateName(status.State)) + } + return nil +} + +func (s *managedService) close() { + _ = s.handle.Close() + _ = s.manager.Disconnect() +} + +// restart sends Start whenever Stop was accepted, detached from the command's cancellation, +// so an interrupt or a timeout never leaves the service stopped. +func (s *managedService) restart(ctx context.Context) error { + if _, err := s.handle.Control(svc.Stop); err != nil { + return fmt.Errorf("stopping service %s: %w", s.name, err) + } + stopErr := s.waitFor(ctx, svc.Stopped, stopTimeout) + if stopErr == nil { + ok("service %s stopped", s.name) + } + startErr := s.start(context.WithoutCancel(ctx)) + return errors.Join(stopErr, startErr) +} + +// start first waits out a stop the caller abandoned: StartService refuses a stopping service. +func (s *managedService) start(ctx context.Context) error { + ctx, cancel := context.WithTimeout(ctx, startTimeout) + defer cancel() + if err := s.waitFor(ctx, svc.Stopped, startTimeout); err != nil { + return err + } + if err := s.handle.Start(); err != nil { + return fmt.Errorf("starting service %s: %w", s.name, err) + } + if err := s.waitFor(ctx, svc.Running, startTimeout); err != nil { + return err + } + ok("service %s running", s.name) + return nil +} + +func (s *managedService) waitFor(ctx context.Context, want svc.State, timeout time.Duration) error { + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + for { + status, err := s.handle.Query() + if err != nil { + return fmt.Errorf("querying service %s: %w", s.name, err) + } + if status.State == want { + return nil + } + select { + case <-ctx.Done(): + return fmt.Errorf("service %s did not reach %s within %s, last state %s: %w", + s.name, stateName(want), timeout, stateName(status.State), ctx.Err()) + case <-time.After(statePollInterval): + } + } +} + +func stateName(state svc.State) string { + switch state { + case svc.Stopped: + return "stopped" + case svc.StartPending: + return "start pending" + case svc.StopPending: + return "stop pending" + case svc.Running: + return "running" + case svc.ContinuePending: + return "continue pending" + case svc.PausePending: + return "pause pending" + case svc.Paused: + return "paused" + } + return fmt.Sprintf("state %d", state) +} From 8c9342ce2d1f6e440d210ae301788baf77ed78b0 Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:01:11 +0300 Subject: [PATCH 04/20] ci: restart the runner's PostgreSQL through site --- .github/workflows/ci.yml | 57 +++++++++++++++++++ .../20261019-site-loopback-and-restart.md | 6 +- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0088918..daa62b9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,6 +28,63 @@ jobs: with: version: v2.12.2 + # The only test of the service restart itself. The runner image ships + # PostgreSQL 17 as service postgresql-x64-17, Stopped and Disabled, and + # openService refuses both, so the step enables and starts it first. The + # first site run finds settings pending and restarts the service; the + # second finds nothing pending and must not restart it. + - name: Restart the runner's PostgreSQL service through site + env: + SEMIBASE_SUPER_PASSWORD: root + SEMIBASE_WRITER_PASSWORD: ci-writer + SEMIBASE_PLOT_PASSWORD: ci-plot + PGPASSWORD: root + run: | + $service = 'postgresql-x64-17' + $psql = "$env:PGBIN\psql.exe" + Get-Service $service | Select-Object Name, Status, StartType | Format-List + Write-Host "PGBIN=$env:PGBIN" + Set-Service $service -StartupType Manual + Start-Service $service + # the first site connect does not retry, and Running may precede the postmaster + $ready = $false + foreach ($attempt in 1..30) { + & "$env:PGBIN\pg_isready.exe" -h 127.0.0.1 -p 5432 + if ($LASTEXITCODE -eq 0) { $ready = $true; break } + Start-Sleep -Seconds 1 + } + if (-not $ready) { Write-Host "::error::$service accepted no connection after Start-Service"; exit 1 } + + go build -o semibase.exe ./cmd/semibase + if ($LASTEXITCODE -ne 0) { exit 1 } + + .\semibase.exe site --port 5432 --expected-major 17 --restart-service $service | + Tee-Object -Variable first + if ($LASTEXITCODE -ne 0) { Write-Host "::error::the first site run exited $LASTEXITCODE"; exit 1 } + # joined: -like over an array filters elements instead of answering yes or no + if (($first -join "`n") -notlike "*restarting service $service*") { + Write-Host "::error::the first site run did not restart $service" + exit 1 + } + + $listen = & $psql -h 127.0.0.1 -U postgres -d postgres -tAc 'SHOW listen_addresses' + if ($LASTEXITCODE -ne 0) { exit 1 } + $pending = & $psql -h 127.0.0.1 -U postgres -d postgres -tAc ` + 'SELECT count(*) FROM pg_settings WHERE pending_restart' + if ($LASTEXITCODE -ne 0) { exit 1 } + if ("$listen".Trim() -ne 'localhost' -or "$pending".Trim() -ne '0') { + Write-Host "::error::listen_addresses '$listen' (want localhost), pending_restart count '$pending' (want 0)" + exit 1 + } + + .\semibase.exe site --port 5432 --expected-major 17 --restart-service $service | + Tee-Object -Variable second + if ($LASTEXITCODE -ne 0) { Write-Host "::error::the second site run exited $LASTEXITCODE"; exit 1 } + if (($second -join "`n") -like '*restarting service*') { + Write-Host "::error::the second site run restarted $service with nothing pending" + exit 1 + } + linux: runs-on: ubuntu-latest env: diff --git a/docs/plans/20261019-site-loopback-and-restart.md b/docs/plans/20261019-site-loopback-and-restart.md index 70119f1..46a8bba 100644 --- a/docs/plans/20261019-site-loopback-and-restart.md +++ b/docs/plans/20261019-site-loopback-and-restart.md @@ -159,11 +159,11 @@ **Files:** - Modify: `.github/workflows/ci.yml` -- [ ] add a step to the Windows job: `Set-Service postgresql-x64-17 -StartupType Manual`, +- [x] add a step to the Windows job: `Set-Service postgresql-x64-17 -StartupType Manual`, `Start-Service postgresql-x64-17` -- [ ] run `semibase site` with `SEMIBASE_SUPER_PASSWORD=root`, writer and plot passwords set +- [x] run `semibase site` with `SEMIBASE_SUPER_PASSWORD=root`, writer and plot passwords set (the first run creates those roles), and the flags from Acceptance Evidence -- [ ] run the three Windows checks from Acceptance Evidence; this step is the only test of the +- [x] run the three Windows checks from Acceptance Evidence; this step is the only test of the service sequence itself - [ ] push the branch and read the CI run - both jobs green before task 5 From 95777165193b24ed2b9d352c50ddaf0067b8480e Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:03:15 +0300 Subject: [PATCH 05/20] docs(plans): record the Windows restart run --- docs/plans/20261019-site-loopback-and-restart.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/plans/20261019-site-loopback-and-restart.md b/docs/plans/20261019-site-loopback-and-restart.md index 46a8bba..3ad2a67 100644 --- a/docs/plans/20261019-site-loopback-and-restart.md +++ b/docs/plans/20261019-site-loopback-and-restart.md @@ -165,7 +165,7 @@ (the first run creates those roles), and the flags from Acceptance Evidence - [x] run the three Windows checks from Acceptance Evidence; this step is the only test of the service sequence itself -- [ ] push the branch and read the CI run - both jobs green before task 5 +- [x] push the branch and read the CI run - both jobs green before task 5 ### Task 5: Verify acceptance criteria - [ ] every item in Acceptance Evidence holds on the CI run From 34d9fda412eac06d0bd87038711b8e81247b61ee Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:09:04 +0300 Subject: [PATCH 06/20] docs(plans): record the acceptance run --- docs/plans/20261019-site-loopback-and-restart.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/plans/20261019-site-loopback-and-restart.md b/docs/plans/20261019-site-loopback-and-restart.md index 3ad2a67..39cca28 100644 --- a/docs/plans/20261019-site-loopback-and-restart.md +++ b/docs/plans/20261019-site-loopback-and-restart.md @@ -168,8 +168,13 @@ - [x] push the branch and read the CI run - both jobs green before task 5 ### Task 5: Verify acceptance criteria -- [ ] every item in Acceptance Evidence holds on the CI run -- [ ] `go test -race ./...`, `go vet ./...`, `golangci-lint run`, `gofmt -l .` prints nothing +- [x] every item in Acceptance Evidence holds on the CI run (run 39249102852 at `9577716`, both + jobs green: Linux `Require the listen_addresses delta` passed; Windows first `site` printed + `restarting service postgresql-x64-17: listen_addresses, shared_buffers waiting for a + restart`, the `SHOW`/`count(*)` check passed, the second run printed no restart line) +- [x] `go test -race ./...`, `go vet ./...`, `golangci-lint run`, `gofmt -l .` prints nothing + (locally `-race` needs cgo and this host has no C compiler, so `go test ./...` ran without + it; both CI jobs run `go test -race ./...` green; `GOOS=linux go vet ./...` clean) ### Task 6: [Final] Update documentation - [ ] `docs/architecture/provisioning.md:160-165`: the opt-in restart, where it runs, the From 9e80374e909d83e926acc3abd249483e031318fa Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:17:53 +0300 Subject: [PATCH 07/20] docs(site): describe loopback and --restart-service --- .github/workflows/release.yml | 2 +- CLAUDE.md | 42 +++++++--- Dockerfile | 2 +- cmd/semibase/main.go | 2 +- docs/architecture/README.md | 5 +- docs/architecture/configuration.md | 33 +++++--- docs/architecture/overview.md | 11 ++- docs/architecture/provisioning.md | 81 ++++++++++++++++--- docs/deployment.md | 26 ++++-- .../20261019-site-loopback-and-restart.md | 17 ++-- internal/provision/provision.go | 6 +- 11 files changed, 165 insertions(+), 62 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7e329d7..605538e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -121,7 +121,7 @@ jobs: echo "$DELIMITER" } >> "$GITHUB_OUTPUT" - # The module is pure pgx with no cgo, so one runner cross-compiles both + # The module is pure Go with no cgo, so one runner cross-compiles both # targets. Native-platform coverage is CI's job, not the release's. - name: Build env: diff --git a/CLAUDE.md b/CLAUDE.md index 568ddb3..5467c68 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,7 +7,8 @@ CLI binary, `semibase.exe`, a container image carrying its Linux build, plus the architecture docs. Deployment target: Windows. Language: Go (module `github.com/Semiteq/SemiBase`), driver `pgx/v5`, the five `sql/semiplot_*.sql` files and `sql/trends.sql` embedded via `go:embed`. Entry point: -`cmd/semibase`. The module is pure `pgx` and compiles for any GOOS; Linux builds serve the +`cmd/semibase`. The module is pure Go (`pgx/v5`, plus `golang.org/x/sys` for the Windows service +restart) and compiles for any GOOS; Linux builds serve the containerised test bench of consumers (SemiPlot runs `bench` against an ephemeral `postgres:17` container). All commands run from the repository root. @@ -24,7 +25,7 @@ Without ldflags, `version` falls back to the VCS revision recorded by the Go too `.github/workflows/release.yml` fires on a `v*` tag push. One `ubuntu-latest` job cross-compiles `windows/amd64` and `linux/amd64` with `CGO_ENABLED=0` — the module is pure -`pgx`, so native runners are CI's job, not the release's — and attaches +Go, so native runners are CI's job, not the release's — and attaches `semibase___amd64[.exe]` to the GitHub release — no extension on the Linux artifact, an ELF is executable by its mode bit. `-X main.revision=` is embedded, so `semibase version` prints the tag. @@ -79,10 +80,17 @@ golangci-lint run golangci-lint is pinned to 2.12.2 (`winget install GolangCI.golangci-lint --version 2.12.2`); config in `.golangci.yml`. CI (`.github/workflows/ci.yml`) runs `go build`, `go test -race`, -and the same lint on `windows-latest` and `ubuntu-latest` for every push and pull request; -the Linux job also provisions a `postgres:17-alpine` service container by running `site` -twice — the tuning path and the idempotency check in one step. -It then runs `bench` against a `postgres:14-alpine` container — the version floor, and the only +and the same lint on `windows-latest` and `ubuntu-latest` for every push and pull request. +The Windows job then runs the `Restart the runner's PostgreSQL service through site` step, the +only test of the service restart: it sets the runner's Stopped and Disabled `postgresql-x64-17` +to Manual and starts it, requires the first `site --restart-service postgresql-x64-17` run to +print `restarting service postgresql-x64-17` and leave `listen_addresses = localhost` with no +setting pending, and requires a second run not to restart the service. +The Linux job provisions a `postgres:17-alpine` service container by running `site` +twice — the tuning path and the idempotency check in one step. The +`Require the listen_addresses delta` step then requires `listen_addresses` still pending: the +container is never restarted, so a pending value proves `site` wrote the delta. +The job then runs `bench` against a `postgres:14-alpine` container — the version floor, and the only place `REVOKE CREATE ON SCHEMA public FROM PUBLIC` bites, because 15 and later withhold that grant by engine default — provisions a third container with `bench` as a `postgres` image init script over the unix socket, and builds the container image, so a broken `Dockerfile` fails on the pull @@ -99,12 +107,13 @@ password. The 14 step starts its server with all five statement-logging paramete `Keep the CREATE ROLE password statements out of the PostgreSQL 14 log` step reads that log. Both log steps run `.github/scripts/check-server-log.sh`, which requires `logging_collector = off`, so `docker logs` is the whole server log, requires the `semiplot` login's probe statement in it, and -requires the passwords absent. The pens and the 17 log steps need `psql` on the runner; to run -them locally, point `psql` at a container. +requires the passwords absent. The `listen_addresses`, the pens and the 17 log steps need `psql` +on the runner; to run them locally, point `psql` at a container. The Windows step uses the +runner's own `$env:PGBIN\psql.exe`. Unit tests live beside the source (`cmd/semibase/*_test.go`, `internal/provision/*_test.go`), table-driven. There are no database-touching tests in the repository; the integration checks are -the `semiplot` access check both commands run at the tail and the six CI database steps above, +the `semiplot` access check both commands run at the tail and the eight CI database steps above, against a live server. ## Format @@ -118,11 +127,12 @@ gofmt -w . # run before presenting changes; gofmt is authoritative ```powershell .\semibase.exe --help # commands: site | bench | version .\semibase.exe site # an installation machine: tuning, then everything else +.\semibase.exe site --restart-service postgresql-x64-17 # also restart the service when settings wait .\semibase.exe bench --port 15432 --database semiplot_dev --expected-major 17 .\semibase.exe version # print the build revision ``` -`site` and `bench` differ in one thing: `site` applies the `ALTER SYSTEM` memory constants, +`site` and `bench` differ in one thing: `site` applies the `ALTER SYSTEM` server settings, `bench` does not. Both create the database, the roles, the grants, the five SemiPlot tables (`semiplot_tags`, `semiplot_groups`, `semiplot_pen_groups`, `semiplot_markers`, `semiplot_meta`), the `semiplot_register_new_pens()` function and `public.trends`, and both end by actually reading @@ -135,6 +145,13 @@ in a rolled-back transaction, requiring 42501 for `INSERT`, `DELETE` and `UPDATE on the archive or on `semiplot_meta` and no `CREATE` on schema `public` (revoked from `PUBLIC` by `create`, since 14 still grants it). A failed check is a non-zero exit. +Two of the settings, `shared_buffers` and `listen_addresses = 'localhost'`, wait for a service +restart. Without a flag the run warns and the operator restarts. `site --restart-service ` +restarts that Windows service inside the tuning phase when settings wait, so `create` and the +tail check run against the restarted server. `bench` refuses the flag, and so does a `--host` +other than `localhost`, `127.0.0.1` or `::1`; the service is checked before anything changes +(`docs/architecture/provisioning.md#the-service-restart`). + The `semiplot` grant on `semiplot_tags` is column-level: `SELECT` and `UPDATE` on the nine settings columns, never `id`, so `has_table_privilege(..., 'UPDATE')` answers false for that table and no check may ask it. @@ -168,8 +185,9 @@ superuser connection, so the table's owner is the role the SCADA writes with and ``` cmd/semibase/ CLI entry point: command dispatch, flags, usage text -internal/provision/ one file per phase behind the two commands: config.go (ALTER SYSTEM), - create.go (db/roles/grants/trends), schema.go (the semiplot schema files, +internal/provision/ one file per phase behind the two commands: config.go (ALTER SYSTEM and + the optional service restart, whose service control lives in + service_windows.go and service_other.go), create.go (db/roles/grants/trends), schema.go (the semiplot schema files, their grants, the pen-registration function's grant and its owner's registrarStatements), check.go (semiplot access). provision.go holds Options, Site/Bench and the connection plumbing they share diff --git a/Dockerfile b/Dockerfile index 5931539..7a4a98a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,7 @@ # The image carries the linux/amd64 binary and nothing else. Its only consumer # copies the binary out (COPY --from=ghcr.io/semiteq/semibase:latest /semibase), # so a base with a shell, CA certificates or a libc would ship weight nobody -# runs: the module is pure pgx, builds CGO_ENABLED=0 and speaks no TLS on the +# runs: the module is pure Go, builds CGO_ENABLED=0 and speaks no TLS on the # bench. The ENTRYPOINT costs one line and makes `docker run version` # a valid smoke check. # diff --git a/cmd/semibase/main.go b/cmd/semibase/main.go index c409499..173f6d2 100644 --- a/cmd/semibase/main.go +++ b/cmd/semibase/main.go @@ -20,7 +20,7 @@ Usage: semibase [flags] Commands: - site an installation machine: apply the memory tuning (ALTER SYSTEM + reload), + site an installation machine: apply the server settings (ALTER SYSTEM + reload), then create the archive database, the roles, the access chain, public.trends, the five semiplot_* tables and semiplot_register_new_pens() bench a throwaway container: the same, without the tuning diff --git a/docs/architecture/README.md b/docs/architecture/README.md index d368b06..45177ec 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -19,10 +19,11 @@ Declarative architecture docs (English, present tense). These describe the syste | Installation | PostgreSQL via `winget`, major version pinned, automatic upgrade disabled; engine install stays outside the tool | | Major version | 17 for new installs, pinned latest; floor 14 (SemiPlot needs `date_bin`; Simple-Scada 2 docs require 14+) | | Provisioning tool | Go single binary `semibase.exe` (`pgx/v5`, embedded SQL); no runtime or `psql` on the target machine | -| Instance configuration | Fixed `ALTER SYSTEM` deltas applied by `semibase site` via `pg_reload_conf()`, never by editing `postgresql.conf`; `shared_buffers` takes effect at the next service restart, reported through `pg_settings.pending_restart` | +| Instance configuration | Fixed `ALTER SYSTEM` deltas applied by `semibase site` via `pg_reload_conf()`, never by editing `postgresql.conf`; `shared_buffers` and `listen_addresses` take effect at the next service restart, reported through `pg_settings.pending_restart`; `site --restart-service ` performs that restart on Windows, opt-in | +| Network | Loopback only (`listen_addresses = 'localhost'`): every client runs on the installation machine | | Hardware floor | Installation machines guarantee 8 GB RAM and 50 GB database disk; memory settings are fixed constants sized to that floor, identical on every machine | | Roles | Four: the superuser the tool connects as, `scada_writer` (SCADA), `semiplot` (viewers), and `semiplot_registrar`, a `NOLOGIN` role that owns `semiplot_register_new_pens()` so its `SECURITY DEFINER` body never runs as the superuser. `semiplot` holds `SELECT` on the archive; `SELECT` on `semiplot_tags` and `UPDATE` on its settings columns, never on `id`; full row access to `semiplot_groups`, `semiplot_pen_groups` and `semiplot_markers`; `SELECT` on `semiplot_meta`; `EXECUTE` on the function; and no `CREATE` on schema `public` — revoked from `PUBLIC` by `create`, since PostgreSQL 14 still grants it by default. The superuser owns the tables, because it is the connection that creates them. One viewer role rather than a second write-only one: both passwords would share one plaintext file, so the split would separate nothing | -| Command surface | Two commands, `site` and `bench`, named for the situation rather than for the tool's internal steps; they differ in one thing, the memory tuning, which only `site` applies | +| Command surface | Two commands, `site` and `bench`, named for the situation rather than for the tool's internal steps; they differ in one thing, the server settings, which only `site` applies | | Archive read access | `ALTER DEFAULT PRIVILEGES FOR ROLE scada_writer` set **before** `public.trends` is created, and before the writer first runs. The `semiplot_*` tables are granted table by table instead, since the superuser creates them | | Objects we add | `semiplot_tags`, `semiplot_groups`, `semiplot_pen_groups`, `semiplot_markers`, `semiplot_meta`, the `semiplot_register_new_pens()` function the viewer calls, and the vendor-shaped `public.trends` — no triggers, scheduled jobs, extensions, or other functions; `messages` is not created, the SCADA makes it | | SemiPlot schema version | `semiplot_meta.schema_version`, `1` in this release, written by `provision` alone. A floor, not an equality: a viewer refuses a database below the version it needs and accepts one above, because the versions this repository issues grow by addition. When the rule starts, and what a schema change does before then: [provisioning.md](./provisioning.md#the-schema-version-is-a-floor) | diff --git a/docs/architecture/configuration.md b/docs/architecture/configuration.md index a297229..8d5ab23 100644 --- a/docs/architecture/configuration.md +++ b/docs/architecture/configuration.md @@ -1,8 +1,8 @@ # Instance configuration Every setting the `site` command writes is listed. Most differ from the PostgreSQL default; -two (`effective_cache_size`, `checkpoint_completion_target`) equal it and are pinned -deliberately, so the values in force are visible here rather than implied. The workload is a +three (`effective_cache_size`, `checkpoint_completion_target`, `listen_addresses`) equal it and +are pinned deliberately, so the values in force are visible here rather than implied. The workload is a sustained append-only insert stream from one writer plus a small number of read-heavy analytical queries. @@ -12,21 +12,25 @@ configuration, and no setting depends on inspecting the host. `semibase site` writes these through `ALTER SYSTEM`, so they live in `postgresql.auto.conf` and survive a reinstall of `postgresql.conf`, and applies them with `pg_reload_conf()`. Every setting -below except `shared_buffers` is reload-context or weaker and takes effect immediately; -`shared_buffers` takes effect at the next service restart or reboot. The tuning phase prints the -server's own pending-restart list (`pg_settings.pending_restart`), and the tail of the run reads it -again as its last statement, so a missed restart is the last thing the run says. `bench` reads it +below except `shared_buffers` and `listen_addresses` is reload-context or weaker and takes effect +immediately; those two take effect at the next service restart or reboot. The tuning phase prints +the server's own pending-restart list (`pg_settings.pending_restart`), and the tail of the run +reads it again as its last statement, so a missed restart is the last thing the run says. +`site --restart-service ` restarts that Windows service inside the tuning phase when the +list is not empty, and the tail then reads it empty +([provisioning.md](./provisioning.md#the-service-restart)). `bench` reads it too — it tunes nothing, but it runs against existing servers, and one that a `site` run tuned can still be waiting for its restart. -`semibase bench` writes none of them: the values are constants sized to the installation -machine's hardware floor and mean nothing to a throwaway container. +`semibase bench` writes none of them: the memory values are constants sized to the installation +machine's hardware floor and mean nothing to a throwaway container, and a loopback-only +`listen_addresses` would shut out the clients that reach a bench through a published port. ## Server settings | Setting | Default | Ours | Why | | --- | --- | --- | --- | -| `shared_buffers` | 128 MB | 2 GB | The archive working set is far larger than the default cache. A quarter of the 8 GB hardware floor; the one setting that needs a service restart. | +| `shared_buffers` | 128 MB | 2 GB | The archive working set is far larger than the default cache. A quarter of the 8 GB hardware floor. Takes effect at a service restart. | | `effective_cache_size` | 4 GB | 4 GB | Planner hint only; set explicitly at half the hardware floor so it never falls below the value the readers' index scans depend on. | | `work_mem` | 4 MB | 64 MB | The readers' pixel-bucket query groups and sorts; at the default it spills to disk on wide windows. | | `maintenance_work_mem` | 64 MB | 512 MB | Index builds and vacuum on daily partitions. | @@ -37,6 +41,7 @@ machine's hardware floor and mean nothing to a throwaway container. | `random_page_cost` | 4.0 | 1.1 | The default assumes a spinning disk and discourages index scans. The machine has a solid-state drive. | | `log_min_duration_statement` | off | 1000 ms | A slow query must leave a trace; the only diagnostic that survives an operator restarting the client. The exception is `semibase`'s own superuser sessions, which set it to -1 to keep password statements out of the log ([provisioning.md](./provisioning.md#invariants-the-provision-package-owns)). | | `track_io_timing` | off | on | Makes `EXPLAIN (ANALYZE, BUFFERS)` usable when a query is slow in the field. | +| `listen_addresses` | `localhost` | `localhost` | Every client of the instance runs on the installation machine ([Network](#network)). Pinned because the installed `postgresql.conf` sets another value: on the Windows runner's PostgreSQL 17 and in the `postgres:17-alpine` container the setting is pending after `site` (measured in CI). Takes effect at a service restart. | Left at defaults deliberately: `max_connections` (a handful of clients), autovacuum (PostgreSQL 13+ already triggers vacuum on insert-only tables, which is what sets the visibility @@ -54,5 +59,11 @@ SCADA's writes: ## Network -The service runs under a dedicated Windows account and listens on the loopback interface plus the -operator network only. +The service runs under a dedicated Windows account and listens on the loopback interface only. +Every client of the instance runs on the same Windows machine: the SCADA, the viewers and +`semibase` itself. Nothing on the operator network may reach the server. + +`site` sets `listen_addresses = 'localhost'` through `ALTER SYSTEM`. The value lands in +`postgresql.auto.conf`, which the server reads after `postgresql.conf`, so it overrides whatever +the installer wrote there. Once the service restarts, a client that names the machine's own IP +address or host name instead of `localhost` or `127.0.0.1` is refused. diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md index 6878028..d5f47c0 100644 --- a/docs/architecture/overview.md +++ b/docs/architecture/overview.md @@ -56,10 +56,13 @@ The order matters, because the default privileges have to be in place before any 1. Install the engine: `winget install --id PostgreSQL.PostgreSQL.17 --exact`. 2. `semibase site` — tuning, then the archive database, the three created roles, default privileges, `public.trends`, the five SemiPlot tables and `semiplot_register_new_pens()`; it ends by proving `semiplot` reads `trends`, writes - what it owns, cannot add, delete or re-key a pen and cannot write the archive, and warns while `shared_buffers` waits for a service - restart. -3. Restart the PostgreSQL service or reboot the machine, so `shared_buffers` takes effect. -4. Point the Simple-Scada project at the database and start it once. It is expected to find + what it owns, cannot add, delete or re-key a pen and cannot write the archive, and warns while `shared_buffers` or + `listen_addresses` waits for a service restart. With `--restart-service ` it restarts that Windows service itself + and continues against the restarted server (`provisioning.md#the-service-restart`). +3. Without `--restart-service`, restart the PostgreSQL service or reboot the machine, so `shared_buffers` and + `listen_addresses` take effect. +4. Point the Simple-Scada project at the database on `localhost` or `127.0.0.1`, since the server listens on loopback + only, and start it once. It is expected to find `trends` in place, write into it, and create the day partitions and `messages` itself — an **unverified** assumption, with the experiment that settles it, in `provisioning.md` ("Assumption: the SCADA meeting an existing `trends`"). Set diff --git a/docs/architecture/provisioning.md b/docs/architecture/provisioning.md index 959e455..56779d9 100644 --- a/docs/architecture/provisioning.md +++ b/docs/architecture/provisioning.md @@ -78,7 +78,7 @@ situations it is run in, not the phases it runs through. | Command | What it does | When | | --- | --- | --- | -| `site` | The memory tuning, then everything else | An installation machine | +| `site` | The server settings, then everything else | An installation machine | | `bench` | Everything else, without the tuning | A throwaway container: a consumer's test bench or a development database | The two differ in exactly one thing, the tuning. The rest is identical, which is what makes a bench @@ -86,12 +86,13 @@ a test of what a site runs: | Step | `site` | `bench` | | --- | --- | --- | -| `ALTER SYSTEM` constants from `configuration.md`, applied with `pg_reload_conf()` | yes | no — the constants are sized for an installation machine and mean nothing to a container | +| `ALTER SYSTEM` deltas from `configuration.md`, applied with `pg_reload_conf()` | yes | no: the memory constants are sized for an installation machine and mean nothing to a container, and a loopback-only `listen_addresses` would shut out the clients that reach a bench through a published port | +| Restarting the Windows service the deltas wait for ([The service restart](#the-service-restart)) | only with `--restart-service` | no: `Validate` refuses the flag | | Archive database, the three created roles, grants, default privileges | yes | yes | | `public.trends` with the `tpdefault` partition, created as `scada_writer` | yes | yes | | The five SemiPlot tables and `semiplot_register_new_pens()`, after `public.trends` because the function body names it | yes | yes | | The `semiplot` access checks at the tail | yes | yes | -| Pending-restart warning at the tail | yes | yes — `bench` tunes nothing, but it runs against existing servers too, and one a `site` run tuned can still be waiting for its restart | +| Pending-restart warning at the tail | yes, and after a `--restart-service` restart the list is empty | yes — `bench` tunes nothing, but it runs against existing servers too, and one a `site` run tuned can still be waiting for its restart | `version` (or `--version`) prints the build revision: the ldflags value (`-X main.revision=...`) when a release sets it, otherwise the VCS revision from @@ -179,13 +180,68 @@ The package that runs the SQL owns its guards; the CLI is a thin dispatcher. - **Cancellable execution.** Every phase honors its context: cancellation stops the in-flight query through pgx instead of killing the process mid-DDL. The context itself — `signal.NotifyContext(..., os.Interrupt)` plus the five-minute per-command timeout - (`phaseTimeout`) — is owned by the CLI dispatcher in `cmd/semibase`. -- **Pending-restart reporting.** The tool never touches the service manager. The tuning phase - applies its deltas with `pg_reload_conf()` and reads `pg_settings.pending_restart` back from - the server; both commands read it again as the **last** statement of the tail, after the - access checks, so a pending `shared_buffers` is named in the run's last line. The operator - restarts the service or reboots the machine — the one manual step, and the tool says so - instead of doing it. + (`phaseTimeout`) — is owned by the CLI dispatcher in `cmd/semibase`. The one exception is the + Start of a service restart, which ignores the command's cancellation so that the service is + never left stopped ([The service restart](#the-service-restart)). +- **Pending-restart reporting.** The tuning phase applies its deltas with `pg_reload_conf()` and + reads `pg_settings.pending_restart` back from the server; both commands read it again as the + **last** statement of the tail, after the access checks, so a pending `shared_buffers` or + `listen_addresses` is named in the run's last line. Without `--restart-service` the tool never + touches the service manager: the operator restarts the service or reboots the machine, and the + tool says so instead of doing it. With the flag, `site` restarts the named Windows service + itself, and the tail reads an empty list. + +## The service restart + +`site --restart-service ` restarts that Windows service when, and only when, the tuning +phase leaves settings waiting for a restart, then waits for the server to accept connections and +continues. Without the flag the run reports the pending settings and changes nothing else. + +`Options.Validate` refuses the flag in two cases. The command is `bench`, which applies no settings, +so nothing it does waits for a restart. The `--host` is not `localhost`, `127.0.0.1` or `::1`, +because after the restart the server listens on the loopback interface only. + +`config` opens the service before it connects, so a service it cannot restart fails the run with +nothing changed. `openService` (`service_windows.go`) refuses a service that does not exist, a +service manager or service the run may not control (the message says to run `semibase` as an +administrator), a Disabled start type, which would leave the service stopped after the Stop, and a +state other than Running, which means the service is not the server the run connects to. Off +Windows, `service_other.go` refuses the flag the same way and names the platform, so the module +stays one cross-platform build. + +The restart runs inside `config`, right after `pendingRestartSettings` returns a non-empty list, so +`create` and `check` run against the restarted server. `applySettings` closes the phase connection +before it returns; then: + +1. Stop, then wait up to 3 minutes for Stopped: a shutdown checkpoint with `max_wal_size = 8GB` can + be long. +2. Start, then wait up to 2 minutes for Running. +3. Connect to `postgres` as the superuser every second, up to 60 s. Whether an EDB service reports + Running before the postmaster accepts connections is unverified; when it does not, the loop + costs one connect. A timeout reports the last connect error. +4. Read `pending_restart` on the new connection. A non-empty list is an error naming the settings. + +Once the service accepts the Stop, Start is always sent. It runs on `context.WithoutCancel` with its +own 2-minute bound, and it first waits out a stop still in progress, because a stopping service +refuses Start. An interrupt, the five-minute command timeout or a failed stop wait therefore never +leaves the service stopped. Every error names the service and its last observed state. + +Service control goes through `golang.org/x/sys/windows/svc/mgr`, not `net stop` and `net start`. +Its errors do not depend on the Windows display language, it reads the start type, so a Disabled +service is refused before any change, and it reports the service state for the bounded waits. +`x/sys` is pure Go, so the release still cross-compiles with `CGO_ENABLED=0`. + +A restart drops every open connection to the server, the SCADA's included. + +CI's Linux job runs `site` without the flag: its server is a GitHub Actions service container, not +a Windows service, and nothing in the job restarts it. The `Require the listen_addresses delta` +step then reads `listen_addresses` still pending, which proves `site` wrote the delta. The Windows +job's `Restart the runner's PostgreSQL service through site` step is the one test of the restart +itself. The runner image ships PostgreSQL 17 as `postgresql-x64-17`, Stopped and Disabled, so the +step sets it to Manual and starts it. It then requires the first +`site --restart-service postgresql-x64-17` run to print `restarting service postgresql-x64-17` +and to leave `listen_addresses = localhost` with no setting pending, and a second run not to +restart the service. ## The roles @@ -492,8 +548,9 @@ promises. They are knowable at exit precisely because this tool creates `public. was given, and on a socket host — `peer` is the platform default there and no consumer reads over the socket. 9. No setting waits for a service restart (`pg_settings.pending_restart`) — a warning, not a - failure, so a pending `shared_buffers` never blocks the access checks. Both commands read it, - last, after the access checks. + failure, so a pending `shared_buffers` or `listen_addresses` never blocks the access checks. + Both commands read it, last, after the access checks. After a `--restart-service` restart it + reads an empty list. ## Development benches diff --git a/docs/deployment.md b/docs/deployment.md index af76fec..21a3b45 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -32,7 +32,7 @@ SemiBase разворачивается одной утилитой — `semibas | Команда | Действие | | --------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | `site` | Машина установки: настройки сервера через `ALTER SYSTEM`, затем архивная база, роли, права доступа, пять таблиц SemiPlot и `public.trends` | -| `bench` | Временный контейнер (тестовый стенд, база для разработки): то же самое без настроек сервера — константы памяти рассчитаны на машину установки и контейнеру ничего не дают | +| `bench` | Временный контейнер (тестовый стенд, база для разработки): то же самое без настроек сервера: память в них рассчитана на машину установки, а прослушивание только локального интерфейса закрыло бы контейнер от клиентов | | `version` | Печать ревизии сборки | Обе команды заканчиваются проверкой: `semiplot` читает `public.trends` и версию схемы, вызывает @@ -156,8 +156,9 @@ Simple-Scada 2 на той же машине, и ей доверяют: она winget install --id PostgreSQL.PostgreSQL.17 --exact ``` -2. Задать пароли ролей переменными окружения в том окне PowerShell, из которого запускается - утилита. Каждая строка запрашивает пароль, и в историю команд PowerShell попадает только +2. Открыть PowerShell от имени администратора и задать в этом окне пароли ролей переменными + окружения: из него же запускается утилита, а перезапуск службы требует прав администратора. + Каждая строка запрашивает пароль, и в историю команд PowerShell попадает только сама строка, без пароля. Переменные живут, пока окно открыто. ```powershell @@ -169,9 +170,12 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па 3. Выполнить одну команду: ```powershell -.\semibase.exe site +.\semibase.exe site --restart-service postgresql-x64-17 ``` +`postgresql-x64-17` — имя службы, которую создаёт установщик PostgreSQL 17. Проверить его можно +командой `Get-Service postgresql*`. + На выходе утилита гарантирует: настройки сервера применены, архивная база, три роли, права и права по умолчанию созданы, пять таблиц SemiPlot, функция `semiplot_register_new_pens()` и `public.trends` (с партицией `tpdefault`) на месте, `semiplot` читает `public.trends`, вызывает @@ -179,11 +183,16 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па удалить или перенумеровать перо и не может писать в архив. Отдельной проверки после этого не требуется — она уже выполнена, её результат в коде возврата. -4. Перезапустить службу PostgreSQL или перезагрузить машину. Все настройки, кроме - `shared_buffers`, применяются сразу; `shared_buffers` — только после перезапуска. - Утилита предупреждает об этом в последних строках вывода, пока перезапуск не сделан. +4. Перезапустить службу PostgreSQL. С флагом `--restart-service` это уже сделала утилита на + шаге 3: если настройки ждут перезапуска, она перезапускает службу, ждёт, пока сервер снова + примет подключения, и продолжает работу. Без флага перезапустите службу или перезагрузите + машину: `shared_buffers` и `listen_addresses` применяются только после перезапуска, остальные + настройки сразу. Пока перезапуск не сделан, утилита предупреждает об этом в последних строках + вывода. Перезапуск разрывает подключения SCADA, поэтому на работающей установке SCADA + останавливают до повторного запуска `site`. -5. Запустить проект Simple-Scada против базы. Мы рассчитываем, что таблицу `trends` он застанет +5. Запустить проект Simple-Scada против базы. Проект подключают к `localhost` или `127.0.0.1`: + после перезапуска сервер принимает подключения только с этой машины. Мы рассчитываем, что таблицу `trends` он застанет готовой, оставит её как есть и дальше создаст дневные партиции и `messages` сам. **Это предположение не проверено на реальном проекте.** На первом запуске включите `log_statement = 'all'` и прочитайте в журнале PostgreSQL, какие DDL-команды SCADA @@ -215,5 +224,6 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па | `--port` | `5432` | Порт сервера | | `--database` | `scada_archive` | Имя архивной базы | | `--expected-major` | не проверяется | Требуемая мажорная версия сервера | +| `--restart-service` | не задан | Только `site` в Windows: служба PostgreSQL, которую утилита перезапускает, если настройки ждут перезапуска. Требует `--host` `localhost`, `127.0.0.1` или `::1` и запуска от имени администратора. Отключённую или остановленную службу утилита не трогает и завершается с ошибкой | Полный перечень — `semibase <команда> --help`. diff --git a/docs/plans/20261019-site-loopback-and-restart.md b/docs/plans/20261019-site-loopback-and-restart.md index 39cca28..a6e4091 100644 --- a/docs/plans/20261019-site-loopback-and-restart.md +++ b/docs/plans/20261019-site-loopback-and-restart.md @@ -177,21 +177,24 @@ it; both CI jobs run `go test -race ./...` green; `GOOS=linux go vet ./...` clean) ### Task 6: [Final] Update documentation -- [ ] `docs/architecture/provisioning.md:160-165`: the opt-in restart, where it runs, the +- [x] `docs/architecture/provisioning.md:160-165`: the opt-in restart, where it runs, the Start guarantee, why CI's Linux job does not use it; `provisioning.md:94` follows -- [ ] `docs/architecture/configuration.md`: `listen_addresses` in the deltas table; + (the bullet now at `:183`, plus a new "The service restart" section and a table row) +- [x] `docs/architecture/configuration.md`: `listen_addresses` in the deltas table; `configuration.md:15-16` and `:29` stop calling `shared_buffers` the only restart setting; `configuration.md:57-58` becomes loopback only, single machine, and notes that `postgresql.auto.conf` overrides `postgresql.conf` -- [ ] `docs/architecture/README.md:22` and `docs/architecture/overview.md:61` (provisioning +- [x] `docs/architecture/README.md:22` and `docs/architecture/overview.md:61` (provisioning step 3) name the flag -- [ ] `docs/deployment.md`, in Russian: the flags table gets `--restart-service`; step 4 says the +- [x] `docs/deployment.md`, in Russian: the flags table gets `--restart-service`; step 4 says the flag does the restart; one sentence that a restart drops the SCADA's connections, so the SCADA is stopped before a re-run on a working site -- [ ] "memory constants" and "memory tuning" become "server settings" in `CLAUDE.md`, +- [x] "memory constants" and "memory tuning" become "server settings" in `CLAUDE.md`, `main.go:26`; "pure `pgx`" becomes "pure Go" in `CLAUDE.md`, `release.yml:124` and - `Dockerfile:3-4`; the Run section of `CLAUDE.md` mentions the flag -- [ ] move this plan to `docs/plans/completed/` + `Dockerfile:3-4`; the Run section of `CLAUDE.md` mentions the flag (also the same phrases + in `provision.go:65,77`, `README.md:25` and `provisioning.md:81`; the CLAUDE.md Test + section gains both new CI steps and counts eight database steps) +- [ ] move this plan to `docs/plans/completed/` (left for the delivery step) ## Post-Completion - On an installation machine, confirm the Simple-Scada project connects to `localhost` or diff --git a/internal/provision/provision.go b/internal/provision/provision.go index 966ff1e..3925388 100644 --- a/internal/provision/provision.go +++ b/internal/provision/provision.go @@ -62,7 +62,7 @@ func (o Options) Validate() error { return nil } -// Site brings an installation machine to its commissioned state: memory tuning, then +// Site brings an installation machine to its commissioned state: server settings, then // everything the archive needs. func (o Options) Site(ctx context.Context) error { if err := o.config(ctx); err != nil { @@ -74,8 +74,8 @@ func (o Options) Site(ctx context.Context) error { return o.check(ctx) } -// Bench brings a throwaway container to the same state minus the tuning: its memory -// constants are sized for an installation machine and mean nothing to a container. +// Bench brings a throwaway container to the same state minus the server settings: they are +// sized for an installation machine, and a loopback-only listener would shut out its clients. func (o Options) Bench(ctx context.Context) error { if err := o.create(ctx); err != nil { return err From 6910e56ae1085b9f6fae3578ace0e89582776c53 Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:42:41 +0300 Subject: [PATCH 08/20] fix(provision): bound and finish the service restart --- cmd/semibase/main.go | 21 ++- cmd/semibase/main_test.go | 59 +++++-- internal/provision/check.go | 5 +- internal/provision/config.go | 60 +++++-- internal/provision/config_test.go | 90 +++++++--- internal/provision/provision.go | 10 +- internal/provision/provision_test.go | 29 ++++ internal/provision/service_other.go | 2 +- internal/provision/service_other_test.go | 3 +- internal/provision/service_windows.go | 173 ++++++++++++++------ internal/provision/service_windows_test.go | 182 +++++++++++++++++++++ 11 files changed, 504 insertions(+), 130 deletions(-) create mode 100644 internal/provision/service_windows_test.go diff --git a/cmd/semibase/main.go b/cmd/semibase/main.go index 173f6d2..9f0b08b 100644 --- a/cmd/semibase/main.go +++ b/cmd/semibase/main.go @@ -113,6 +113,10 @@ func run(ctx context.Context, arguments []string) int { defer cancel() if err := execute(commandContext, options); err != nil { fmt.Fprintln(errorOutput, "error:", err) + if errors.Is(commandContext.Err(), context.DeadlineExceeded) { + fmt.Fprintf(errorOutput, "%s ran out of its %s limit; every step checks before it creates, "+ + "so re-running it continues from where it stopped\n", command, phaseTimeout) + } return 1 } fmt.Fprintf(output, "\nDone: %s completed against %s.\n", @@ -151,10 +155,9 @@ func revisionFromSettings(settings []debug.BuildSetting) string { return vcsRevision } -// One flag set serves both commands, which differ only in the tuning phase. The tuning -// phase's one flag, --restart-service, is refused for bench by Options.Validate. -// Password flags default to empty so the usage output can only ever show the variable -// names, never their values. +// One flag set serves both commands, which differ only in the tuning phase; its one flag, +// --restart-service, is registered for site alone. Password flags default to empty so the +// usage output can only ever show the variable names, never their values. func newFlagSet(command string, options *provision.Options) *flag.FlagSet { flags := flag.NewFlagSet(command, flag.ContinueOnError) flags.SetOutput(errorOutput) @@ -171,14 +174,16 @@ func newFlagSet(command string, options *provision.Options) *flag.FlagSet { "semiplot password (env SEMIBASE_PLOT_PASSWORD)") flags.IntVar(&options.ExpectedMajor, "expected-major", 0, "required server major version; 0 accepts any major >= 14") - flags.StringVar(&options.RestartService, "restart-service", "", - "site only: Windows service to restart when the server settings wait for a restart; "+ - "needs a loopback --host") + if command == "site" { + flags.StringVar(&options.RestartService, "restart-service", "", + "Windows service to restart when the server settings wait for a restart; "+ + "needs a loopback --host") + } return flags } func parseOptions(command string, arguments []string) (provision.Options, error) { - options := provision.Options{Command: command} + options := provision.Options{} flags := newFlagSet(command, &options) if err := flags.Parse(arguments); err != nil { return provision.Options{}, err diff --git a/cmd/semibase/main_test.go b/cmd/semibase/main_test.go index 0ca4005..5851902 100644 --- a/cmd/semibase/main_test.go +++ b/cmd/semibase/main_test.go @@ -90,6 +90,7 @@ func TestParseOptions(t *testing.T) { environment map[string]string wantErr error wantAnyErr bool + wantErrText string check func(t *testing.T, options provision.Options) }{ { @@ -101,21 +102,22 @@ func TestParseOptions(t *testing.T) { }, }, { - name: "bench rejects --restart-service", - arguments: []string{"--restart-service", "x"}, - wantAnyErr: true, + name: "bench has no --restart-service", + command: "bench", + arguments: []string{"--restart-service", "x"}, + wantErrText: "flag provided but not defined: -restart-service", }, { - name: "site rejects --restart-service with a non-loopback host", - command: "site", - arguments: []string{"--restart-service", "x", "--host", "db.internal"}, - wantAnyErr: true, + name: "site rejects --restart-service with a non-loopback host", + command: "site", + arguments: []string{"--restart-service", "x", "--host", "db.internal"}, + wantErrText: "loopback", }, { - name: "site rejects --restart-service with a socket host", - command: "site", - arguments: []string{"--restart-service", "x", "--host", "/var/run/postgresql"}, - wantAnyErr: true, + name: "site rejects --restart-service with a socket host", + command: "site", + arguments: []string{"--restart-service", "x", "--host", "/var/run/postgresql"}, + wantErrText: "loopback", }, { name: "site accepts --restart-service with the default host", @@ -180,13 +182,13 @@ func TestParseOptions(t *testing.T) { t.Setenv(name, value) } - command := tt.command - if command == "" { - command = "bench" - } - options, err := parseOptions(command, tt.arguments) + options, err := parseOptions(tt.command, tt.arguments) switch { + case tt.wantErrText != "": + if err == nil || !strings.Contains(err.Error(), tt.wantErrText) { + t.Fatalf("parseOptions error = %v, want one containing %q", err, tt.wantErrText) + } case tt.wantErr != nil: if !errors.Is(err, tt.wantErr) { t.Fatalf("parseOptions error = %v, want %v", err, tt.wantErr) @@ -370,6 +372,28 @@ func TestRunReportsTheEndpointItRanAgainst(t *testing.T) { } } +func TestRunNamesTheUsedUpTimeLimit(t *testing.T) { + buffer := captureErrorOutput(t) + clearPasswordEnvironment(t) + + // mutates the shared commands map, so no t.Parallel here + commands["stub-deadline-exceeded"] = func(ctx context.Context, _ provision.Options) error { + <-ctx.Done() + return ctx.Err() + } + t.Cleanup(func() { delete(commands, "stub-deadline-exceeded") }) + + // an expired parent deadline stands in for a command that used up phaseTimeout + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + defer cancel() + if got := run(ctx, []string{"stub-deadline-exceeded"}); got != 1 { + t.Fatalf("run(stub-deadline-exceeded) = %d, want exit code 1", got) + } + if want := "ran out of its " + phaseTimeout.String() + " limit"; !strings.Contains(buffer.String(), want) { + t.Errorf("error output %q does not contain %q", buffer.String(), want) + } +} + func TestRunReportsCommandFailure(t *testing.T) { buffer := captureErrorOutput(t) clearPasswordEnvironment(t) @@ -386,6 +410,9 @@ func TestRunReportsCommandFailure(t *testing.T) { if !strings.Contains(buffer.String(), "error: stub-failure sentinel") { t.Errorf("error output does not report the failure: %q", buffer.String()) } + if strings.Contains(buffer.String(), "limit") { + t.Errorf("error output blames the time limit for an ordinary failure: %q", buffer.String()) + } } func TestCancelledContextReturnsPromptly(t *testing.T) { diff --git a/internal/provision/check.go b/internal/provision/check.go index 26b8d3c..afe03eb 100644 --- a/internal/provision/check.go +++ b/internal/provision/check.go @@ -163,7 +163,7 @@ func (o Options) check(ctx context.Context) error { if err := o.checkPlotLogin(ctx); err != nil { return err } - // last, so a pending shared_buffers is the last thing the run prints + // last, so a setting still waiting for a restart is the last thing the run prints return reportPendingRestart(ctx, archive) } @@ -418,8 +418,7 @@ func reportPendingRestart(ctx context.Context, conn *pgx.Conn) error { return err } if len(pending) > 0 { - warn("settings waiting for a service restart: %s - restart the PostgreSQL service or reboot the machine.", - strings.Join(pending, ", ")) + warn("settings waiting for a service restart: %s. %s", strings.Join(pending, ", "), restartAdvice(pending)) } return nil } diff --git a/internal/provision/config.go b/internal/provision/config.go index e184a9d..37ad566 100644 --- a/internal/provision/config.go +++ b/internal/provision/config.go @@ -2,11 +2,14 @@ package provision import ( "context" + "errors" "fmt" + "slices" "strings" "time" "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" ) type setting struct { @@ -44,7 +47,7 @@ func (o Options) config(ctx context.Context) error { if o.RestartService != "" { var err error // SA4023: off Windows openService always fails, by design - if service, err = openService(o.RestartService); err != nil { //nolint:staticcheck + if service, err = openService(ctx, o.RestartService); err != nil { //nolint:staticcheck return err } defer service.close() @@ -55,11 +58,10 @@ func (o Options) config(ctx context.Context) error { return err } switch { - case restartNeeded(pending, o.RestartService): + case len(pending) > 0 && service != nil: return o.restartService(ctx, service, pending) case len(pending) > 0: - note("waiting for a service restart: %s. Restart the PostgreSQL service or reboot the machine.", - strings.Join(pending, ", ")) + note("waiting for a service restart: %s. %s", strings.Join(pending, ", "), restartAdvice(pending)) default: ok("all settings active") } @@ -95,8 +97,13 @@ func (o Options) applySettings(ctx context.Context) ([]string, error) { return pendingRestartSettings(ctx, conn) } -func restartNeeded(pending []string, service string) bool { - return len(pending) > 0 && service != "" +// restartAdvice is the operator's next step for settings that wait for a restart. +func restartAdvice(pending []string) string { + advice := "Restart the PostgreSQL service or reboot the machine." + if slices.Contains(pending, "listen_addresses") { + advice += " After the restart the server accepts connections from this machine only." + } + return advice } func (o Options) restartService(ctx context.Context, service *managedService, pending []string) error { @@ -118,14 +125,17 @@ func (o Options) restartService(ctx context.Context, service *managedService, pe return err } if len(stillPending) > 0 { - return fmt.Errorf("after restarting service %s, settings still wait for a restart: %s", - o.RestartService, strings.Join(stillPending, ", ")) + return fmt.Errorf("after restarting service %s, settings still wait for a restart: %s. "+ + "The service may belong to another server than the one on port %d: compare the -D in "+ + "its PathName (Get-CimInstance Win32_Service) with SHOW data_directory", + o.RestartService, strings.Join(stillPending, ", "), o.Port) } ok("all settings active") return nil } -// waitReady dials at once, then every interval, until a dial succeeds or timeout elapses. +// waitReady dials at once, then every interval, until a dial succeeds or timeout elapses. Network +// errors and 57P03 are retried; any other server error is final. func waitReady( ctx context.Context, timeout, interval time.Duration, @@ -133,22 +143,46 @@ func waitReady( ) (*pgx.Conn, error) { waitCtx, cancel := context.WithTimeout(ctx, timeout) defer cancel() + var lastErr error for { conn, err := dial(waitCtx) if err == nil { return conn, nil } + if waitCtx.Err() != nil { + // a dial cut short by the deadline reports the context, not why the server refused + if lastErr == nil { + lastErr = err + } + return nil, waitReadyFailure(ctx, timeout, lastErr) + } + if !retryableConnectError(err) { + return nil, err + } + lastErr = err select { case <-waitCtx.Done(): - if ctx.Err() != nil { - return nil, fmt.Errorf("waiting for the server to accept connections: %w", ctx.Err()) - } - return nil, fmt.Errorf("the server accepted no connection within %s: %w", timeout, err) + return nil, waitReadyFailure(ctx, timeout, lastErr) case <-time.After(interval): } } } +func waitReadyFailure(ctx context.Context, timeout time.Duration, lastErr error) error { + if ctx.Err() != nil { + return fmt.Errorf("waiting for the server to accept connections: %w; last connect error: %w", + ctx.Err(), lastErr) + } + return fmt.Errorf("the server accepted no connection within %s: %w", timeout, lastErr) +} + +const cannotConnectNow = "57P03" + +func retryableConnectError(err error) bool { + var pgErr *pgconn.PgError + return !errors.As(err, &pgErr) || pgErr.Code == cannotConnectNow +} + func pendingRestartSettings(ctx context.Context, conn *pgx.Conn) ([]string, error) { rows, err := conn.Query(ctx, "SELECT name FROM pg_settings WHERE pending_restart ORDER BY name") if err != nil { diff --git a/internal/provision/config_test.go b/internal/provision/config_test.go index 7773afa..126470f 100644 --- a/internal/provision/config_test.go +++ b/internal/provision/config_test.go @@ -3,28 +3,32 @@ package provision import ( "context" "errors" + "strings" "testing" "time" "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgconn" ) -func TestRestartNeeded(t *testing.T) { +func TestRestartAdvice(t *testing.T) { + const loopbackNote = "from this machine only" tests := []struct { - name string - pending []string - service string - want bool + name string + pending []string + wantLoopback bool }{ - {"nothing pending, no service", nil, "", false}, - {"nothing pending, service named", nil, "postgresql-x64-17", false}, - {"pending, no service", []string{"shared_buffers"}, "", false}, - {"pending, service named", []string{"listen_addresses", "shared_buffers"}, "postgresql-x64-17", true}, + {"shared_buffers alone", []string{"shared_buffers"}, false}, + {"listen_addresses pending", []string{"listen_addresses", "shared_buffers"}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := restartNeeded(tt.pending, tt.service); got != tt.want { - t.Errorf("restartNeeded(%v, %q) = %v, want %v", tt.pending, tt.service, got, tt.want) + got := restartAdvice(tt.pending) + if !strings.Contains(got, "Restart the PostgreSQL service") { + t.Errorf("restartAdvice(%v) = %q, want the restart instruction", tt.pending, got) + } + if hasNote := strings.Contains(got, loopbackNote); hasNote != tt.wantLoopback { + t.Errorf("restartAdvice(%v) = %q, loopback note %v, want %v", tt.pending, got, hasNote, tt.wantLoopback) } }) } @@ -32,13 +36,14 @@ func TestRestartNeeded(t *testing.T) { var errRefused = errors.New("connection refused") -// failingDialer returns a dialer that fails failures times, then returns conn, and its call count. -func failingDialer(failures int, conn *pgx.Conn) (func(context.Context) (*pgx.Conn, error), *int) { +// scriptedDialer returns a dialer that fails with failure failures times, then returns conn, and +// its call count. +func scriptedDialer(failures int, failure error, conn *pgx.Conn) (func(context.Context) (*pgx.Conn, error), *int) { calls := 0 return func(context.Context) (*pgx.Conn, error) { calls++ if calls <= failures { - return nil, errRefused + return nil, failure } return conn, nil }, &calls @@ -46,24 +51,38 @@ func failingDialer(failures int, conn *pgx.Conn) (func(context.Context) (*pgx.Co func TestWaitReady(t *testing.T) { conn := &pgx.Conn{} + wrongPassword := &pgconn.PgError{Code: "28P01", Message: "password authentication failed"} + startingUp := &pgconn.PgError{Code: cannotConnectNow, Message: "the database system is starting up"} tests := []struct { name string failures int + failure error timeout time.Duration - wantCalls int - wantErr bool + wantErr error + wantCalls func(calls int) bool }{ - {"succeeds at once", 0, time.Second, 1, false}, - {"succeeds after three failures", 3, time.Second, 4, false}, - {"times out with the last error", 1 << 30, 50 * time.Millisecond, 0, true}, + {"succeeds at once", 0, nil, time.Second, nil, func(calls int) bool { return calls == 1 }}, + {"succeeds after three refusals", 3, errRefused, time.Second, nil, func(calls int) bool { return calls == 4 }}, + {"retries 57P03", 2, startingUp, time.Second, nil, func(calls int) bool { return calls == 3 }}, + { + "times out with the last connect error", 1 << 30, errRefused, 50 * time.Millisecond, errRefused, + func(calls int) bool { return calls > 1 }, + }, + { + "gives up at once on another server error", 1 << 30, wrongPassword, time.Minute, wrongPassword, + func(calls int) bool { return calls == 1 }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - dial, calls := failingDialer(tt.failures, conn) + dial, calls := scriptedDialer(tt.failures, tt.failure, conn) got, err := waitReady(context.Background(), tt.timeout, time.Millisecond, dial) - if tt.wantErr { - if !errors.Is(err, errRefused) { - t.Fatalf("waitReady() error = %v, want it to wrap %v", err, errRefused) + if !tt.wantCalls(*calls) { + t.Errorf("dial called %d times", *calls) + } + if tt.wantErr != nil { + if !errors.Is(err, tt.wantErr) { + t.Fatalf("waitReady() error = %v, want it to wrap %v", err, tt.wantErr) } if got != nil { t.Errorf("waitReady() returned a connection with error %v", err) @@ -76,13 +95,27 @@ func TestWaitReady(t *testing.T) { if got != conn { t.Errorf("waitReady() returned %p, want the dialed connection %p", got, conn) } - if *calls != tt.wantCalls { - t.Errorf("dial called %d times, want %d", *calls, tt.wantCalls) - } }) } } +// a dial in flight at the deadline fails with the context's error; the message keeps the refusal +func TestWaitReadyKeepsTheLastRealErrorAtTheDeadline(t *testing.T) { + calls := 0 + dial := func(ctx context.Context) (*pgx.Conn, error) { + calls++ + if calls == 1 { + return nil, errRefused + } + <-ctx.Done() + return nil, ctx.Err() + } + _, err := waitReady(context.Background(), 50*time.Millisecond, time.Millisecond, dial) + if !errors.Is(err, errRefused) { + t.Fatalf("waitReady() error = %v, want it to wrap %v", err, errRefused) + } +} + func TestWaitReadyStopsOnCancellation(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) calls := 0 @@ -97,7 +130,10 @@ func TestWaitReadyStopsOnCancellation(t *testing.T) { if !errors.Is(err, context.Canceled) { t.Fatalf("waitReady() error = %v, want it to wrap %v", err, context.Canceled) } + if !errors.Is(err, errRefused) { + t.Errorf("waitReady() error = %v, want it to keep the last connect error %v", err, errRefused) + } if calls != 2 { - t.Errorf("dial called %d times after the cancellation, want 2", calls) + t.Errorf("dial called %d times, want 2: no dial after the cancellation", calls) } } diff --git a/internal/provision/provision.go b/internal/provision/provision.go index 3925388..68d1aea 100644 --- a/internal/provision/provision.go +++ b/internal/provision/provision.go @@ -27,7 +27,6 @@ const versionFloor = 140000 const archiveTable = "public.trends" type Options struct { - Command string Host string Port int Database string @@ -47,14 +46,7 @@ func (o Options) Validate() error { if !databaseNamePattern.MatchString(o.Database) { return fmt.Errorf("database name %q must match %s", o.Database, databaseNamePattern) } - if o.RestartService == "" { - return nil - } - if o.Command != "site" { - return fmt.Errorf("--restart-service is accepted by site only: %s applies no server "+ - "settings, so nothing it does waits for a restart", o.Command) - } - if !slices.Contains(loopbackHosts, o.Host) { + if o.RestartService != "" && !slices.Contains(loopbackHosts, o.Host) { return fmt.Errorf("--restart-service needs --host to be one of %s, not %q: after the "+ "restart the server listens on the loopback interface only", strings.Join(loopbackHosts, ", "), o.Host) diff --git a/internal/provision/provision_test.go b/internal/provision/provision_test.go index 5ce7ecb..2ad59cc 100644 --- a/internal/provision/provision_test.go +++ b/internal/provision/provision_test.go @@ -10,6 +10,35 @@ import ( "github.com/jackc/pgx/v5/pgconn" ) +func TestValidateRestartServiceHost(t *testing.T) { + tests := []struct { + host string + wantErr bool + }{ + {"localhost", false}, + {"127.0.0.1", false}, + {"::1", false}, + {"db.internal", true}, + {"192.168.1.10", true}, + {"/var/run/postgresql", true}, + } + for _, tt := range tests { + t.Run(tt.host, func(t *testing.T) { + options := Options{Database: "scada_archive", Host: tt.host, RestartService: "postgresql-x64-17"} + err := options.Validate() + if gotErr := err != nil; gotErr != tt.wantErr { + t.Errorf("Validate() with host %q: error = %v, wantErr %v", tt.host, err, tt.wantErr) + } + if err != nil && !strings.Contains(err.Error(), "loopback") { + t.Errorf("Validate() error = %q, want it to name the loopback rule", err) + } + }) + } + if err := (Options{Database: "scada_archive", Host: "db.internal"}).Validate(); err != nil { + t.Errorf("Validate() without --restart-service refused a remote host: %v", err) + } +} + func TestValidate(t *testing.T) { tests := []struct { name string diff --git a/internal/provision/service_other.go b/internal/provision/service_other.go index 22afdd7..b949fc1 100644 --- a/internal/provision/service_other.go +++ b/internal/provision/service_other.go @@ -11,7 +11,7 @@ import ( type managedService struct{} -func openService(name string) (*managedService, error) { +func openService(_ context.Context, name string) (*managedService, error) { return nil, fmt.Errorf("--restart-service %s: service restart is not supported on %s", name, runtime.GOOS) } diff --git a/internal/provision/service_other_test.go b/internal/provision/service_other_test.go index f743c76..c698176 100644 --- a/internal/provision/service_other_test.go +++ b/internal/provision/service_other_test.go @@ -3,13 +3,14 @@ package provision import ( + "context" "runtime" "strings" "testing" ) func TestOpenServiceIsRefusedOffWindows(t *testing.T) { - service, err := openService("postgresql-x64-17") + service, err := openService(context.Background(), "postgresql-x64-17") if err == nil { service.close() t.Fatal("openService() succeeded off Windows") diff --git a/internal/provision/service_windows.go b/internal/provision/service_windows.go index cd0d349..aaf7973 100644 --- a/internal/provision/service_windows.go +++ b/internal/provision/service_windows.go @@ -13,22 +13,34 @@ import ( "golang.org/x/sys/windows/svc/mgr" ) -const ( +// serviceControl is the part of *mgr.Service the restart drives, so tests can script the states. +type serviceControl interface { + Query() (svc.Status, error) + Control(c svc.Cmd) (svc.Status, error) + Start(args ...string) error +} + +type serviceTimings struct { // a shutdown checkpoint with max_wal_size = 8GB can be long - stopTimeout = 3 * time.Minute - startTimeout = 2 * time.Minute - statePollInterval = 500 * time.Millisecond -) + stop time.Duration + start time.Duration + poll time.Duration +} + +func defaultServiceTimings() serviceTimings { + return serviceTimings{stop: 3 * time.Minute, start: 2 * time.Minute, poll: 500 * time.Millisecond} +} type managedService struct { name string - manager *mgr.Mgr - handle *mgr.Service + control serviceControl + timings serviceTimings + release func() } // openService refuses a service the restart could not complete: missing, out of reach, -// Disabled, or not running, which means it is not the server the run connects to. -func openService(name string) (*managedService, error) { +// Disabled, or not running once a start in progress has finished. +func openService(ctx context.Context, name string) (*managedService, error) { manager, err := mgr.Connect() if err != nil { if errors.Is(err, windows.ERROR_ACCESS_DENIED) { @@ -48,86 +60,143 @@ func openService(name string) (*managedService, error) { } return nil, fmt.Errorf("opening service %s: %w", name, err) } - service := &managedService{name: name, manager: manager, handle: handle} - if err := service.checkRestartable(); err != nil { + service := &managedService{ + name: name, + control: handle, + timings: defaultServiceTimings(), + release: func() { + _ = handle.Close() + _ = manager.Disconnect() + }, + } + if err := refuseDisabled(handle, name); err != nil { + service.close() + return nil, err + } + if err := service.awaitRunning(ctx); err != nil { service.close() return nil, err } return service, nil } -func (s *managedService) checkRestartable() error { - config, err := s.handle.Config() +func refuseDisabled(handle *mgr.Service, name string) error { + config, err := handle.Config() if err != nil { - return fmt.Errorf("reading the configuration of service %s: %w", s.name, err) + return fmt.Errorf("reading the configuration of service %s: %w", name, err) } if config.StartType == mgr.StartDisabled { return fmt.Errorf("service %s is Disabled, so it could not be started again after the stop; "+ - "set its start type to Automatic or Manual", s.name) - } - status, err := s.handle.Query() - if err != nil { - return fmt.Errorf("querying service %s: %w", s.name, err) + "set its start type to Automatic or Manual", name) } - if status.State != svc.Running { - return fmt.Errorf("service %s is %s, not running, so it is not the server this run "+ - "connects to", s.name, stateName(status.State)) + return nil +} + +// awaitRunning lets a start in progress finish: an installer may run site right after it started +// the service, and an EDB service reports Running only once the server accepts connections. +func (s *managedService) awaitRunning(ctx context.Context) error { + waitCtx, cancel := context.WithTimeout(ctx, s.timings.start) + defer cancel() + state, err := s.pollUntil(waitCtx, func(state svc.State) bool { return state != svc.StartPending }) + switch { + case errors.Is(err, context.DeadlineExceeded): + return fmt.Errorf("service %s is still start pending after %s: %w", s.name, s.timings.start, err) + case err != nil: + return err + case state != svc.Running: + return fmt.Errorf("service %s is %s, not running; site restarts a running service only", + s.name, stateName(state)) } return nil } func (s *managedService) close() { - _ = s.handle.Close() - _ = s.manager.Disconnect() + s.release() } -// restart sends Start whenever Stop was accepted, detached from the command's cancellation, -// so an interrupt or a timeout never leaves the service stopped. +// restart detaches from the command's cancellation once the Stop is accepted: the Stopped wait, +// the Start and the Running wait each run to their own bound, so an interrupt or the command +// timeout does not leave the service stopped. Residual cases: docs/architecture/provisioning.md. func (s *managedService) restart(ctx context.Context) error { - if _, err := s.handle.Control(svc.Stop); err != nil { + if _, err := s.control.Control(svc.Stop); err != nil { return fmt.Errorf("stopping service %s: %w", s.name, err) } - stopErr := s.waitFor(ctx, svc.Stopped, stopTimeout) - if stopErr == nil { - ok("service %s stopped", s.name) - } - startErr := s.start(context.WithoutCancel(ctx)) - return errors.Join(stopErr, startErr) -} - -// start first waits out a stop the caller abandoned: StartService refuses a stopping service. -func (s *managedService) start(ctx context.Context) error { - ctx, cancel := context.WithTimeout(ctx, startTimeout) - defer cancel() - if err := s.waitFor(ctx, svc.Stopped, startTimeout); err != nil { + detached := context.WithoutCancel(ctx) + if err := s.awaitStopped(detached); err != nil { return err } - if err := s.handle.Start(); err != nil { - return fmt.Errorf("starting service %s: %w", s.name, err) + ok("service %s stopped", s.name) + if err := s.control.Start(); err != nil { + return fmt.Errorf("starting service %s: %w; it stays stopped: start it with Start-Service %s", + s.name, err, s.name) } - if err := s.waitFor(ctx, svc.Running, startTimeout); err != nil { + if err := s.awaitStarted(detached); err != nil { return err } ok("service %s running", s.name) + if err := ctx.Err(); err != nil { + return fmt.Errorf("service %s restarted, then the run stopped: %w", s.name, err) + } + return nil +} + +func (s *managedService) awaitStopped(ctx context.Context) error { + waitCtx, cancel := context.WithTimeout(ctx, s.timings.stop) + defer cancel() + state, err := s.pollUntil(waitCtx, func(state svc.State) bool { return state == svc.Stopped }) + switch { + case errors.Is(err, context.DeadlineExceeded): + return fmt.Errorf("service %s is still %s %s after the Stop and will stay stopped; once "+ + "Get-Service %s shows Stopped, start it with Start-Service %s: %w", + s.name, stateName(state), s.timings.stop, s.name, s.name, err) + case err != nil: + return fmt.Errorf("%w after the Stop; check it with Get-Service %s and, if it is stopped, "+ + "start it with Start-Service %s", err, s.name, s.name) + } return nil } -func (s *managedService) waitFor(ctx context.Context, want svc.State, timeout time.Duration) error { - ctx, cancel := context.WithTimeout(ctx, timeout) +// awaitStarted fails at once on Stopped from the second query on: a postmaster that cannot start +// on the new settings exits, and the service falls back to Stopped. +func (s *managedService) awaitStarted(ctx context.Context) error { + waitCtx, cancel := context.WithTimeout(ctx, s.timings.start) defer cancel() + queries := 0 + state, err := s.pollUntil(waitCtx, func(state svc.State) bool { + queries++ + return state == svc.Running || (state == svc.Stopped && queries > 1) + }) + switch { + case errors.Is(err, context.DeadlineExceeded): + return fmt.Errorf("service %s did not reach running within %s, last state %s: %w", + s.name, s.timings.start, stateName(state), err) + case err != nil: + return err + case state == svc.Stopped: + return fmt.Errorf("service %s stopped again after the Start: the server did not start on the "+ + "new settings, which stay in postgresql.auto.conf. Read the server log in the data "+ + "directory's log folder and the Windows Application event log", s.name) + } + return nil +} + +// pollUntil queries the service until done accepts its state or ctx ends, and returns the last +// state it saw. +func (s *managedService) pollUntil(ctx context.Context, done func(svc.State) bool) (svc.State, error) { + var last svc.State for { - status, err := s.handle.Query() + status, err := s.control.Query() if err != nil { - return fmt.Errorf("querying service %s: %w", s.name, err) + return last, fmt.Errorf("querying service %s: %w", s.name, err) } - if status.State == want { - return nil + last = status.State + if done(last) { + return last, nil } select { case <-ctx.Done(): - return fmt.Errorf("service %s did not reach %s within %s, last state %s: %w", - s.name, stateName(want), timeout, stateName(status.State), ctx.Err()) - case <-time.After(statePollInterval): + return last, ctx.Err() + case <-time.After(s.timings.poll): } } } diff --git a/internal/provision/service_windows_test.go b/internal/provision/service_windows_test.go new file mode 100644 index 0000000..394d669 --- /dev/null +++ b/internal/provision/service_windows_test.go @@ -0,0 +1,182 @@ +//go:build windows + +package provision + +import ( + "context" + "errors" + "strings" + "testing" + "time" + + "golang.org/x/sys/windows/svc" +) + +// fakeService answers Query with states in order and repeats the last one. +type fakeService struct { + states []svc.State + queries int + controlErr error + startErr error + stops int + starts int +} + +func (f *fakeService) Query() (svc.Status, error) { + state := f.states[min(f.queries, len(f.states)-1)] + f.queries++ + return svc.Status{State: state}, nil +} + +func (f *fakeService) Control(svc.Cmd) (svc.Status, error) { + f.stops++ + return svc.Status{State: svc.StopPending}, f.controlErr +} + +func (f *fakeService) Start(...string) error { + f.starts++ + return f.startErr +} + +func newFakeManagedService(fake *fakeService) *managedService { + return &managedService{ + name: "pg", + control: fake, + timings: serviceTimings{stop: 50 * time.Millisecond, start: 50 * time.Millisecond, poll: time.Millisecond}, + release: func() {}, + } +} + +func cancelledContext() context.Context { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + return ctx +} + +func expiredContext() context.Context { + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + cancel() + return ctx +} + +var fullRestart = []svc.State{svc.StopPending, svc.StopPending, svc.Stopped, svc.StartPending, svc.Running} + +func TestRestart(t *testing.T) { + tests := []struct { + name string + ctx context.Context + states []svc.State + controlErr error + startErr error + wantStarts int + wantErr []string + wantCause error + }{ + {name: "stops and starts", ctx: context.Background(), states: fullRestart, wantStarts: 1}, + { + name: "an interrupt after the Stop still starts the service", ctx: cancelledContext(), + states: fullRestart, wantStarts: 1, wantErr: []string{"restarted, then the run stopped"}, + wantCause: context.Canceled, + }, + { + name: "the command timeout after the Stop still starts the service", ctx: expiredContext(), + states: fullRestart, wantStarts: 1, wantErr: []string{"restarted, then the run stopped"}, + wantCause: context.DeadlineExceeded, + }, + { + name: "a refused Stop changes nothing", ctx: context.Background(), states: []svc.State{svc.Running}, + controlErr: errors.New("access denied"), wantErr: []string{"stopping service pg"}, + }, + { + name: "a stop past its bound says the service will stay stopped", ctx: context.Background(), + states: []svc.State{svc.StopPending}, wantErr: []string{"will stay stopped", "Start-Service pg"}, + wantCause: context.DeadlineExceeded, + }, + { + name: "a refused Start says how to start it", ctx: context.Background(), + states: []svc.State{svc.Stopped}, startErr: errors.New("start refused"), wantStarts: 1, + wantErr: []string{"stays stopped", "Start-Service pg"}, + }, + { + name: "a server that falls back to Stopped fails at once", ctx: context.Background(), + states: []svc.State{svc.Stopped, svc.StartPending, svc.Stopped}, wantStarts: 1, + wantErr: []string{"stopped again after the Start", "postgresql.auto.conf"}, + }, + { + name: "Stopped right after the Start is given one more query", ctx: context.Background(), + states: []svc.State{svc.Stopped, svc.Stopped, svc.Running}, wantStarts: 1, + }, + { + name: "a start past its bound names the last state", ctx: context.Background(), + states: []svc.State{svc.Stopped, svc.StartPending}, wantStarts: 1, + wantErr: []string{"did not reach running", "start pending"}, wantCause: context.DeadlineExceeded, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fake := &fakeService{states: tt.states, controlErr: tt.controlErr, startErr: tt.startErr} + err := newFakeManagedService(fake).restart(tt.ctx) + if fake.stops != 1 { + t.Errorf("Stop sent %d times, want 1", fake.stops) + } + if fake.starts != tt.wantStarts { + t.Errorf("Start sent %d times, want %d", fake.starts, tt.wantStarts) + } + assertServiceError(t, err, tt.wantErr, tt.wantCause) + }) + } +} + +func TestAwaitRunning(t *testing.T) { + tests := []struct { + name string + states []svc.State + wantErr []string + wantCause error + }{ + {name: "running", states: []svc.State{svc.Running}}, + {name: "a start in progress is waited out", states: []svc.State{svc.StartPending, svc.StartPending, svc.Running}}, + { + name: "a start past its bound is refused", states: []svc.State{svc.StartPending}, + wantErr: []string{"still start pending"}, wantCause: context.DeadlineExceeded, + }, + {name: "stopped is refused", states: []svc.State{svc.Stopped}, wantErr: []string{"is stopped, not running"}}, + {name: "stopping is refused", states: []svc.State{svc.StopPending}, wantErr: []string{"is stop pending"}}, + {name: "paused is refused", states: []svc.State{svc.Paused}, wantErr: []string{"is paused"}}, + { + name: "a start that fails is refused", states: []svc.State{svc.StartPending, svc.Stopped}, + wantErr: []string{"is stopped, not running"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fake := &fakeService{states: tt.states} + err := newFakeManagedService(fake).awaitRunning(context.Background()) + assertServiceError(t, err, tt.wantErr, tt.wantCause) + if fake.stops != 0 || fake.starts != 0 { + t.Errorf("the check sent %d Stop and %d Start, want none", fake.stops, fake.starts) + } + }) + } +} + +func assertServiceError(t *testing.T, err error, wantText []string, wantCause error) { + t.Helper() + if wantText == nil { + if err != nil { + t.Fatalf("error = %v, want nil", err) + } + return + } + if err == nil { + t.Fatalf("error = nil, want one containing %q", wantText) + } + for _, text := range wantText { + if !strings.Contains(err.Error(), text) { + t.Errorf("error = %q, want it to contain %q", err, text) + } + } + if wantCause != nil && !errors.Is(err, wantCause) { + t.Errorf("error = %v, want it to wrap %v", err, wantCause) + } +} From f19d7fff09f60fdb3a8f48956eb452f03512280e Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:42:41 +0300 Subject: [PATCH 09/20] ci: exercise refusals and a start-pending service --- .github/workflows/ci.yml | 77 +++++++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index daa62b9..afe60cc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,11 +28,15 @@ jobs: with: version: v2.12.2 - # The only test of the service restart itself. The runner image ships - # PostgreSQL 17 as service postgresql-x64-17, Stopped and Disabled, and - # openService refuses both, so the step enables and starts it first. The - # first site run finds settings pending and restarts the service; the - # second finds nothing pending and must not restart it. + # The only test of the service sequence itself. The runner image ships + # PostgreSQL 17 as service postgresql-x64-17, Stopped and Disabled, so the + # first run must refuse it with nothing changed. sc.exe start returns + # while the service is still start pending, so the restart run begins + # with openService waiting for Running, the way an installer that has + # just started the service runs site; no other readiness wait precedes + # it. That run finds settings pending and restarts the service; the next + # finds nothing pending and must not restart it; a service that does not + # exist is refused before any change. - name: Restart the runner's PostgreSQL service through site env: SEMIBASE_SUPER_PASSWORD: root @@ -42,27 +46,37 @@ jobs: run: | $service = 'postgresql-x64-17' $psql = "$env:PGBIN\psql.exe" - Get-Service $service | Select-Object Name, Status, StartType | Format-List - Write-Host "PGBIN=$env:PGBIN" - Set-Service $service -StartupType Manual - Start-Service $service - # the first site connect does not retry, and Running may precede the postmaster - $ready = $false - foreach ($attempt in 1..30) { - & "$env:PGBIN\pg_isready.exe" -h 127.0.0.1 -p 5432 - if ($LASTEXITCODE -eq 0) { $ready = $true; break } - Start-Sleep -Seconds 1 + Get-CimInstance Win32_Service -Filter "Name='$service'" | + Select-Object Name, State, StartMode, PathName | Format-List + go build -o semibase.exe ./cmd/semibase + if ($LASTEXITCODE -ne 0) { exit 1 } + + # stdout and stderr joined into one string: -like over an array filters + # elements instead of answering yes or no + function Invoke-Site([string] $name) { + $ErrorActionPreference = 'Continue' + $lines = & .\semibase.exe site --port 5432 --expected-major 17 --restart-service $name 2>&1 | + ForEach-Object { "$_" } + $lines | Write-Host + [pscustomobject]@{ Code = $LASTEXITCODE; Output = $lines -join "`n" } + } + function Assert-Refused($run, [string] $reason) { + if ($run.Code -ne 1 -or $run.Output -notlike "*$reason*" -or $run.Output -like '*ALTER SYSTEM SET*') { + Write-Host "::error::want exit 1 with '$reason' and no ALTER SYSTEM SET, got exit $($run.Code)" + exit 1 + } } - if (-not $ready) { Write-Host "::error::$service accepted no connection after Start-Service"; exit 1 } - go build -o semibase.exe ./cmd/semibase + Assert-Refused (Invoke-Site $service) "service $service is Disabled" + + Set-Service $service -StartupType Manual + sc.exe start $service if ($LASTEXITCODE -ne 0) { exit 1 } + Write-Host "state after sc.exe start: $((Get-Service $service).Status)" - .\semibase.exe site --port 5432 --expected-major 17 --restart-service $service | - Tee-Object -Variable first - if ($LASTEXITCODE -ne 0) { Write-Host "::error::the first site run exited $LASTEXITCODE"; exit 1 } - # joined: -like over an array filters elements instead of answering yes or no - if (($first -join "`n") -notlike "*restarting service $service*") { + $first = Invoke-Site $service + if ($first.Code -ne 0) { Write-Host "::error::the first site run exited $($first.Code)"; exit 1 } + if ($first.Output -notlike "*restarting service $service*") { Write-Host "::error::the first site run did not restart $service" exit 1 } @@ -77,14 +91,15 @@ jobs: exit 1 } - .\semibase.exe site --port 5432 --expected-major 17 --restart-service $service | - Tee-Object -Variable second - if ($LASTEXITCODE -ne 0) { Write-Host "::error::the second site run exited $LASTEXITCODE"; exit 1 } - if (($second -join "`n") -like '*restarting service*') { + $second = Invoke-Site $service + if ($second.Code -ne 0) { Write-Host "::error::the second site run exited $($second.Code)"; exit 1 } + if ($second.Output -like '*restarting service*') { Write-Host "::error::the second site run restarted $service with nothing pending" exit 1 } + Assert-Refused (Invoke-Site 'no-such-service') 'service no-such-service does not exist' + linux: runs-on: ubuntu-latest env: @@ -128,7 +143,8 @@ jobs: ./semibase site --database semiplot_dev --expected-major 17 # The service container is never restarted, so the loopback-only value - # stays pending; a pending value proves site wrote the delta. + # stays pending; a pending value proves site wrote a delta, and the + # postgresql.auto.conf line proves it wrote 'localhost'. - name: Require the listen_addresses delta env: PGHOST: localhost @@ -138,8 +154,11 @@ jobs: set -euo pipefail pending=$(psql -d postgres -tAc \ "SELECT pending_restart FROM pg_settings WHERE name = 'listen_addresses'") - if [[ "$pending" != t ]]; then - echo "::error::listen_addresses pending_restart is '$pending' (want t)" + written=$(psql -d postgres -tAc \ + "SELECT setting FROM pg_file_settings WHERE name = 'listen_addresses' + AND sourcefile LIKE '%postgresql.auto.conf'") + if [[ "$pending" != t || "$written" != localhost ]]; then + echo "::error::listen_addresses pending_restart '$pending' (want t), auto.conf value '$written' (want localhost)" exit 1 fi From c2af598dc7ac3b4dc2a04a9963e3cd4072b71776 Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:42:42 +0300 Subject: [PATCH 10/20] docs(site): state the restart guarantee and gaps --- CLAUDE.md | 13 ++-- README.md | 5 +- docs/architecture/README.md | 2 +- docs/architecture/configuration.md | 8 +- docs/architecture/provisioning.md | 61 ++++++++++----- docs/deployment.md | 33 ++++---- .../20261019-site-loopback-and-restart.md | 77 ++++++++++++------- 7 files changed, 126 insertions(+), 73 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5467c68..b63758f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -148,9 +148,11 @@ on the archive or on `semiplot_meta` and no `CREATE` on schema `public` (revoked Two of the settings, `shared_buffers` and `listen_addresses = 'localhost'`, wait for a service restart. Without a flag the run warns and the operator restarts. `site --restart-service ` restarts that Windows service inside the tuning phase when settings wait, so `create` and the -tail check run against the restarted server. `bench` refuses the flag, and so does a `--host` -other than `localhost`, `127.0.0.1` or `::1`; the service is checked before anything changes -(`docs/architecture/provisioning.md#the-service-restart`). +tail check run against the restarted server. Only `site` registers the flag, and it refuses a +`--host` other than `localhost`, `127.0.0.1` or `::1`; the service is checked before anything +changes, and once it accepts the Stop the restart runs to completion despite an interrupt +(`docs/architecture/provisioning.md#the-service-restart`, which also lists the cases that still +leave it stopped). The `semiplot` grant on `semiplot_tags` is column-level: `SELECT` and `UPDATE` on the nine settings columns, never `id`, so `has_table_privilege(..., 'UPDATE')` answers false for that table and no @@ -187,8 +189,9 @@ superuser connection, so the table's owner is the role the SCADA writes with and cmd/semibase/ CLI entry point: command dispatch, flags, usage text internal/provision/ one file per phase behind the two commands: config.go (ALTER SYSTEM and the optional service restart, whose service control lives in - service_windows.go and service_other.go), create.go (db/roles/grants/trends), schema.go (the semiplot schema files, - their grants, the pen-registration function's grant and its owner's + service_windows.go and service_other.go), create.go + (db/roles/grants/trends), schema.go (the semiplot schema files, their + grants, the pen-registration function's grant and its owner's registrarStatements), check.go (semiplot access). provision.go holds Options, Site/Bench and the connection plumbing they share sql/ embedded DDL: the five semiplot_* files (ours: the tables, the diff --git a/README.md b/README.md index 59bde3c..2b6a7d7 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,7 @@ SemiBase — разворачиваемый PostgreSQL-сервис для ус # установка pgsql winget install --id PostgreSQL.PostgreSQL.17 --exact +# дальше всё в PowerShell от имени администратора: утилита перезапускает службу PostgreSQL # пароли ролей: каждая строка запрашивает пароль, в историю PowerShell он не попадает $env:SEMIBASE_SUPER_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Пароль postgres' -AsSecureString)).Password $env:SEMIBASE_WRITER_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Пароль scada_writer' -AsSecureString)).Password @@ -36,6 +37,6 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па # конфигурация сервера, архивная база, роли `scada_writer`, `semiplot` и # `semiplot_registrar`, права, таблица архива, пять таблиц SemiPlot и функция -# регистрации новых перьев -.\semibase.exe site +# регистрации новых перьев; перезапуск службы, если настройки его ждут +.\semibase.exe site --restart-service postgresql-x64-17 ``` diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 45177ec..9b053c8 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -18,7 +18,7 @@ Declarative architecture docs (English, present tense). These describe the syste | --- | --- | | Installation | PostgreSQL via `winget`, major version pinned, automatic upgrade disabled; engine install stays outside the tool | | Major version | 17 for new installs, pinned latest; floor 14 (SemiPlot needs `date_bin`; Simple-Scada 2 docs require 14+) | -| Provisioning tool | Go single binary `semibase.exe` (`pgx/v5`, embedded SQL); no runtime or `psql` on the target machine | +| Provisioning tool | Go single binary `semibase.exe` (`pgx/v5`, `golang.org/x/sys` for the Windows service, embedded SQL); no runtime or `psql` on the target machine | | Instance configuration | Fixed `ALTER SYSTEM` deltas applied by `semibase site` via `pg_reload_conf()`, never by editing `postgresql.conf`; `shared_buffers` and `listen_addresses` take effect at the next service restart, reported through `pg_settings.pending_restart`; `site --restart-service ` performs that restart on Windows, opt-in | | Network | Loopback only (`listen_addresses = 'localhost'`): every client runs on the installation machine | | Hardware floor | Installation machines guarantee 8 GB RAM and 50 GB database disk; memory settings are fixed constants sized to that floor, identical on every machine | diff --git a/docs/architecture/configuration.md b/docs/architecture/configuration.md index 8d5ab23..ada0cf6 100644 --- a/docs/architecture/configuration.md +++ b/docs/architecture/configuration.md @@ -1,8 +1,10 @@ # Instance configuration -Every setting the `site` command writes is listed. Most differ from the PostgreSQL default; -three (`effective_cache_size`, `checkpoint_completion_target`, `listen_addresses`) equal it and -are pinned deliberately, so the values in force are visible here rather than implied. The workload is a +Every setting the `site` command writes is listed. Most differ from the PostgreSQL default. +Two (`effective_cache_size`, `checkpoint_completion_target`) equal it and are pinned +deliberately, so the values in force are visible here rather than implied. `listen_addresses` +equals the compiled default too, and its pin is what changes the server: it overrides the value +the installed `postgresql.conf` sets ([Network](#network)). The workload is a sustained append-only insert stream from one writer plus a small number of read-heavy analytical queries. diff --git a/docs/architecture/provisioning.md b/docs/architecture/provisioning.md index 56779d9..050d7eb 100644 --- a/docs/architecture/provisioning.md +++ b/docs/architecture/provisioning.md @@ -87,7 +87,7 @@ a test of what a site runs: | Step | `site` | `bench` | | --- | --- | --- | | `ALTER SYSTEM` deltas from `configuration.md`, applied with `pg_reload_conf()` | yes | no: the memory constants are sized for an installation machine and mean nothing to a container, and a loopback-only `listen_addresses` would shut out the clients that reach a bench through a published port | -| Restarting the Windows service the deltas wait for ([The service restart](#the-service-restart)) | only with `--restart-service` | no: `Validate` refuses the flag | +| Restarting the Windows service the deltas wait for ([The service restart](#the-service-restart)) | only with `--restart-service` | no: `bench` does not register the flag | | Archive database, the three created roles, grants, default privileges | yes | yes | | `public.trends` with the `tpdefault` partition, created as `scada_writer` | yes | yes | | The five SemiPlot tables and `semiplot_register_new_pens()`, after `public.trends` because the function body names it | yes | yes | @@ -180,9 +180,11 @@ The package that runs the SQL owns its guards; the CLI is a thin dispatcher. - **Cancellable execution.** Every phase honors its context: cancellation stops the in-flight query through pgx instead of killing the process mid-DDL. The context itself — `signal.NotifyContext(..., os.Interrupt)` plus the five-minute per-command timeout - (`phaseTimeout`) — is owned by the CLI dispatcher in `cmd/semibase`. The one exception is the - Start of a service restart, which ignores the command's cancellation so that the service is - never left stopped ([The service restart](#the-service-restart)). + (`phaseTimeout`) — is owned by the CLI dispatcher in `cmd/semibase`. The one exception is a + service restart: once the service accepts the Stop, the waits and the Start ignore the + command's cancellation, so that an interrupt does not leave the service stopped + ([The service restart](#the-service-restart)). A run that used up `phaseTimeout` says so in a + second error line. - **Pending-restart reporting.** The tuning phase applies its deltas with `pg_reload_conf()` and reads `pg_settings.pending_restart` back from the server; both commands read it again as the **last** statement of the tail, after the access checks, so a pending `shared_buffers` or @@ -197,34 +199,51 @@ The package that runs the SQL owns its guards; the CLI is a thin dispatcher. phase leaves settings waiting for a restart, then waits for the server to accept connections and continues. Without the flag the run reports the pending settings and changes nothing else. -`Options.Validate` refuses the flag in two cases. The command is `bench`, which applies no settings, -so nothing it does waits for a restart. The `--host` is not `localhost`, `127.0.0.1` or `::1`, -because after the restart the server listens on the loopback interface only. +Only `site` registers the flag: `bench` applies no settings, so nothing it does waits for a +restart, and `bench --restart-service` fails as an undefined flag. `Options.Validate` refuses a +`--host` other than `localhost`, `127.0.0.1` or `::1`, because after the restart the server listens +on the loopback interface only. `config` opens the service before it connects, so a service it cannot restart fails the run with nothing changed. `openService` (`service_windows.go`) refuses a service that does not exist, a service manager or service the run may not control (the message says to run `semibase` as an -administrator), a Disabled start type, which would leave the service stopped after the Stop, and a -state other than Running, which means the service is not the server the run connects to. Off -Windows, `service_other.go` refuses the flag the same way and names the platform, so the module -stays one cross-platform build. +administrator), and a Disabled start type, which would leave the service stopped after the Stop. +A service in Start Pending gets up to 2 minutes to reach Running: an installer may run `site` right +after it started the service, and an EDB service (`pg_ctl runservice ... -w`) reports Running only +once the server accepts connections. Stopped, Stop Pending, Paused, or a start that ends in Stopped, +is refused. The tool does not check that the service is the server behind `--port`: a wrong name +restarts another running instance, and the run then fails with the settings still pending and a +hint to compare the service's `-D` with `SHOW data_directory`. Off Windows, `service_other.go` +refuses the flag the same way and names the platform, so the module stays one cross-platform +build. The restart runs inside `config`, right after `pendingRestartSettings` returns a non-empty list, so `create` and `check` run against the restarted server. `applySettings` closes the phase connection before it returns; then: -1. Stop, then wait up to 3 minutes for Stopped: a shutdown checkpoint with `max_wal_size = 8GB` can - be long. -2. Start, then wait up to 2 minutes for Running. -3. Connect to `postgres` as the superuser every second, up to 60 s. Whether an EDB service reports - Running before the postmaster accepts connections is unverified; when it does not, the loop - costs one connect. A timeout reports the last connect error. +1. Stop, then wait up to 3 minutes from the Stop for Stopped: a shutdown checkpoint with + `max_wal_size = 8GB` can be long. +2. Start, then wait up to 2 minutes from the Start for Running. Stopped after the first query fails + at once: the postmaster exited on the new settings. +3. Connect to `postgres` as the superuser every second, up to 60 s. A refused connection and 57P03 + are retried; any other server error, a wrong password for one, fails at once. A timeout + reports the last connect error, never the context's. 4. Read `pending_restart` on the new connection. A non-empty list is an error naming the settings. -Once the service accepts the Stop, Start is always sent. It runs on `context.WithoutCancel` with its -own 2-minute bound, and it first waits out a stop still in progress, because a stopping service -refuses Start. An interrupt, the five-minute command timeout or a failed stop wait therefore never -leaves the service stopped. Every error names the service and its last observed state. +Once the service accepts the Stop, steps 1 and 2 run on `context.WithoutCancel`, each to its own +bound. An interrupt or the command timeout during them does not stop the sequence: the service is +started, and the run then ends with the cancellation. Each wait error names the service and the +last state it observed. Three cases still leave the service stopped: + +| Case | What the run says | Recovery | +| --- | --- | --- | +| The stop takes longer than 3 minutes | the service is still stopping and will stay stopped | once `Get-Service ` shows Stopped, `Start-Service ` | +| The process ends without running its code: console window closed, logoff, shutdown, a killed process. `signal.NotifyContext` catches `os.Interrupt` only, and Windows ends a console process a few seconds after a close event, which no handler can extend | nothing | `Start-Service ` | +| The postmaster refuses the new settings, for example it cannot allocate `shared_buffers = 2GB`, and the service falls back to Stopped | the service stopped again after the Start; the settings stay in `postgresql.auto.conf` | read the server log in the data directory's `log` folder, remove the offending line from `postgresql.auto.conf`, then `Start-Service ` | + +The command's five-minute `phaseTimeout` also covers the restart. A stop and a start that use +their whole bounds leave no time for the ready wait, `create` and `check`; the run then fails with +the service running, says it ran out of its limit, and a re-run finishes the work. Service control goes through `golang.org/x/sys/windows/svc/mgr`, not `net stop` and `net start`. Its errors do not depend on the Windows display language, it reads the start type, so a Disabled diff --git a/docs/deployment.md b/docs/deployment.md index 21a3b45..a17fa97 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -183,13 +183,20 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па удалить или перенумеровать перо и не может писать в архив. Отдельной проверки после этого не требуется — она уже выполнена, её результат в коде возврата. -4. Перезапустить службу PostgreSQL. С флагом `--restart-service` это уже сделала утилита на - шаге 3: если настройки ждут перезапуска, она перезапускает службу, ждёт, пока сервер снова +4. Убедиться, что служба перезапущена. `shared_buffers` и `listen_addresses` применяются только + после перезапуска службы, остальные настройки сразу. С флагом `--restart-service` утилита на + шаге 3 перезапускает службу сама, если настройки ждут перезапуска, ждёт, пока сервер снова примет подключения, и продолжает работу. Без флага перезапустите службу или перезагрузите - машину: `shared_buffers` и `listen_addresses` применяются только после перезапуска, остальные - настройки сразу. Пока перезапуск не сделан, утилита предупреждает об этом в последних строках - вывода. Перезапуск разрывает подключения SCADA, поэтому на работающей установке SCADA - останавливают до повторного запуска `site`. + машину; пока перезапуск не сделан, утилита предупреждает об этом в последних строках вывода. + + Если утилита сообщает, что служба остановлена и такой останется, запустите её командой + `Start-Service postgresql-x64-17`. Если служба остановилась сразу после запуска, сервер не + принял новые настройки: причина записана в журнале сервера, в папке `log` каталога данных. + + Перезапуск разрывает подключения SCADA. Повторный запуск `site` на работающей установке + перезапускает службу, только если какие-то настройки ждут перезапуска. Их список выводит + запрос `SELECT name FROM pg_settings WHERE pending_restart`; если он не пуст, остановите SCADA + перед запуском. 5. Запустить проект Simple-Scada против базы. Проект подключают к `localhost` или `127.0.0.1`: после перезапуска сервер принимает подключения только с этой машины. Мы рассчитываем, что таблицу `trends` он застанет @@ -218,12 +225,12 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па ## Основные флаги -| Флаг | По умолчанию | Назначение | -| ------------------ | --------------- | ------------------------------------------------------------- | -| `--host` | `localhost` | Адрес сервера или каталог unix-сокета (`/var/run/postgresql`) | -| `--port` | `5432` | Порт сервера | -| `--database` | `scada_archive` | Имя архивной базы | -| `--expected-major` | не проверяется | Требуемая мажорная версия сервера | -| `--restart-service` | не задан | Только `site` в Windows: служба PostgreSQL, которую утилита перезапускает, если настройки ждут перезапуска. Требует `--host` `localhost`, `127.0.0.1` или `::1` и запуска от имени администратора. Отключённую или остановленную службу утилита не трогает и завершается с ошибкой | +| Флаг | По умолчанию | Назначение | +| ------------------- | --------------- | ------------------------------------------------------------- | +| `--host` | `localhost` | Адрес сервера или каталог unix-сокета (`/var/run/postgresql`) | +| `--port` | `5432` | Порт сервера | +| `--database` | `scada_archive` | Имя архивной базы | +| `--expected-major` | не проверяется | Требуемая мажорная версия сервера | +| `--restart-service` | не задан | Только `site` в Windows: служба PostgreSQL, которую утилита перезапускает, если настройки ждут перезапуска. Требует `--host` `localhost`, `127.0.0.1` или `::1` и запуска от имени администратора. Отключённую или остановленную службу утилита не трогает и завершается с ошибкой; только что запущенную службу ждёт до двух минут | Полный перечень — `semibase <команда> --help`. diff --git a/docs/plans/20261019-site-loopback-and-restart.md b/docs/plans/20261019-site-loopback-and-restart.md index a6e4091..9278adc 100644 --- a/docs/plans/20261019-site-loopback-and-restart.md +++ b/docs/plans/20261019-site-loopback-and-restart.md @@ -47,16 +47,23 @@ ## Acceptance Evidence - Linux CI, after the two `site` runs (`ci.yml:70-71`): `PGHOST=localhost PGUSER=postgres PGPASSWORD=ci-super psql -d postgres -tAc "SELECT pending_restart FROM pg_settings WHERE name = 'listen_addresses'"` - prints `t`. The container is never restarted, so the value stays pending; this proves the delta - is written. + prints `t`, and `SELECT setting FROM pg_file_settings WHERE name = 'listen_addresses' AND + sourcefile LIKE '%postgresql.auto.conf'` prints `localhost`. The container is never restarted, + so the value stays pending; this proves the delta is written. - Windows CI, new step against the runner's PostgreSQL 17, with `$psql = "$env:PGBIN\psql.exe"`: 1. `semibase site --port 5432 --expected-major 17 --restart-service postgresql-x64-17` exits 0, and its output contains `restarting service postgresql-x64-17`. 2. `& $psql -h 127.0.0.1 -U postgres -d postgres -tAc "SHOW listen_addresses"` prints `localhost`, and `SELECT count(*) FROM pg_settings WHERE pending_restart` prints `0`. 3. A second identical run exits 0, and its output does not contain `restarting service`. + 4. `site --restart-service` against the shipped Disabled service, and against + `no-such-service`, exits 1 with no `ALTER SYSTEM SET` line in its output. The restart run + follows `sc.exe start` with no other readiness wait, so it begins while the service may + still be Start Pending. - Unit tests: `bench --restart-service x` is a usage error; `--restart-service` with a - non-loopback `--host` is a usage error; `site --restart-service x` with a loopback host parses. + non-loopback `--host` is a usage error; `site --restart-service x` with a loopback host parses; + the restart sends Start after the Stop when the command context is already cancelled or + expired. ## Solution Overview - `listen_addresses` joins the `settings` list. It is postmaster-context, so it lands in @@ -68,36 +75,50 @@ `net stop`/`net start`: its errors do not depend on the Windows display language, it reads the service's start type, so a Disabled service is refused before any change, and it exposes the service state for bounded waits. `x/sys` is pure Go, so cross-compilation without cgo stays. -- The flag parses on every platform. The service is opened and checked at the start of - `config`, before it connects: a wrong name, a missing right, a Disabled start type, or a - non-Windows build (`service_other.go` returns "service restart is not supported on ") - fails with nothing changed. The module stays one cross-platform build: the Linux binary is what - the docker benches run. -- Once Stop has been sent, Start is always sent, on a context detached from the command's - cancellation and timeout (`context.WithoutCancel`) with its own bound. An interrupt or a timeout - during the restart never leaves the service stopped. -- After the service reports Running, the tool reconnects in a bounded loop. Whether an EDB - service reports Running before the postmaster accepts connections is unverified; the loop costs - one successful connect when it does not. +- The flag parses on every platform, for `site` only. The service is opened and checked at the + start of `config`, before it connects: a wrong name, a missing right, a Disabled start type, a + state other than Running once a Start Pending has finished, or a non-Windows build + (`service_other.go` returns "service restart is not supported on ") fails with nothing + changed. The module stays one cross-platform build: the Linux binary is what the docker benches + run. +- Once the service accepts the Stop, the Stopped wait, the Start and the Running wait run on a + context detached from the command's cancellation and timeout (`context.WithoutCancel`), each + wait with its own bound. An interrupt or the command timeout during them does not stop the + sequence. Three cases still leave the service stopped, each with its recovery in + `docs/architecture/provisioning.md#the-service-restart`: a stop longer than its bound, a process + ended by the console close, logoff, shutdown or a kill, and a postmaster that refuses the new + settings. +- After the service reports Running, the tool reconnects in a bounded loop. An EDB service runs + `pg_ctl runservice ... -w` and reports Running only once the postmaster accepts connections + (read with CIM on the development machine), so the loop normally costs one connect. ## Technical Details - `Options.RestartService string`; empty means no restart. -- `--restart-service `, accepted by `site` only. `Validate` refuses it when the command - is `bench` (no tuning, so nothing it does creates a pending restart) and when `--host` is not - `localhost`, `127.0.0.1` or `::1`: after the restart the server listens on loopback only. -- Decision function `restartNeeded(pending []string, service string) bool`: true only when both - are non-empty. +- `--restart-service `, registered by `site` only: `bench` has no tuning, so nothing it + does creates a pending restart, and the flag is undefined for it. `Validate` refuses it when + `--host` is not `localhost`, `127.0.0.1` or `::1`: after the restart the server listens on + loopback only. +- The restart runs when the pending list is non-empty and a service was opened; the condition + sits inline in `config`'s switch. +- `openService` waits up to 2 minutes for a Start Pending service to reach Running, then refuses + any state other than Running. - Restart sequence in `config`: 1. Close the phase connection. - 2. `Stop`, then wait for `Stopped`, bounded by 3 minutes: a shutdown checkpoint with - `max_wal_size = 8GB` can be long. - 3. `Start` on `context.WithoutCancel(ctx)`, then wait for `Running`, bounded by 2 minutes. This - step runs whether or not the stop wait succeeded. - 4. The ready wait connects to `postgres` as the superuser every second, bounded by 60 s. It - returns the new connection, and the last connect error goes into the timeout message. - 5. `pendingRestartSettings` runs on the new connection. A non-empty list is an error naming the - settings. -- Errors name the service and its last observed state. + 2. `Stop` on the command context. From here on, steps 3 and 4 run on + `context.WithoutCancel(ctx)`. + 3. Wait for `Stopped`, bounded by 3 minutes from the Stop: a shutdown checkpoint with + `max_wal_size = 8GB` can be long. If the wait gives up, Start is not sent and the error says + the service will stay stopped and how to start it. + 4. `Start`, then wait for `Running`, bounded by 2 minutes from the Start. Stopped from the + second query on fails at once and points to the server log. + 5. The ready wait connects to `postgres` as the superuser every second, bounded by 60 s. It + retries network errors and 57P03 only, returns the new connection, and the last connect + error that is not a context error goes into the timeout message. + 6. `pendingRestartSettings` runs on the new connection. A non-empty list is an error naming the + settings, with a hint that the service may belong to another server. +- Service control goes through a three-method seam over `*mgr.Service` (`Query`, `Control`, + `Start`), so the sequence is unit-tested with scripted states. +- The wait errors name the service and its last observed state. - Output: `step("restarting service %s: %s waiting for a restart", name, list)`, then `ok` lines for stopped, running, accepting connections. From 0205789a31794f542db6c779f6923eb903436ad7 Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:45:07 +0300 Subject: [PATCH 11/20] ci: end the restart step on its own verdict --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index afe60cc..9852691 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -99,6 +99,8 @@ jobs: } Assert-Refused (Invoke-Site 'no-such-service') 'service no-such-service does not exist' + # the runner exits with the last native exit code, which the refusal left at 1 + exit 0 linux: runs-on: ubuntu-latest From b5529957bc8d907edb125972fc10aa9a1b010f90 Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:45:07 +0300 Subject: [PATCH 12/20] docs(site): record the runner's -w service command --- docs/architecture/provisioning.md | 5 +++-- docs/plans/20261019-site-loopback-and-restart.md | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/architecture/provisioning.md b/docs/architecture/provisioning.md index 050d7eb..977ca0d 100644 --- a/docs/architecture/provisioning.md +++ b/docs/architecture/provisioning.md @@ -209,8 +209,9 @@ nothing changed. `openService` (`service_windows.go`) refuses a service that doe service manager or service the run may not control (the message says to run `semibase` as an administrator), and a Disabled start type, which would leave the service stopped after the Stop. A service in Start Pending gets up to 2 minutes to reach Running: an installer may run `site` right -after it started the service, and an EDB service (`pg_ctl runservice ... -w`) reports Running only -once the server accepts connections. Stopped, Stop Pending, Paused, or a start that ends in Stopped, +after it started the service, and an EDB service (`pg_ctl runservice ... -w`, the `PathName` the +Windows CI step prints) reports Running only once the server accepts connections. That step starts +the service with `sc.exe start`, which returns while it is Start Pending, and runs `site` at once. Stopped, Stop Pending, Paused, or a start that ends in Stopped, is refused. The tool does not check that the service is the server behind `--port`: a wrong name restarts another running instance, and the run then fails with the settings still pending and a hint to compare the service's `-D` with `SHOW data_directory`. Off Windows, `service_other.go` diff --git a/docs/plans/20261019-site-loopback-and-restart.md b/docs/plans/20261019-site-loopback-and-restart.md index 9278adc..2cf44be 100644 --- a/docs/plans/20261019-site-loopback-and-restart.md +++ b/docs/plans/20261019-site-loopback-and-restart.md @@ -90,7 +90,9 @@ settings. - After the service reports Running, the tool reconnects in a bounded loop. An EDB service runs `pg_ctl runservice ... -w` and reports Running only once the postmaster accepts connections - (read with CIM on the development machine), so the loop normally costs one connect. + (read with CIM on the development machine, and printed by the Windows CI step for + `postgresql-x64-17`: `pg_ctl.exe runservice -N "postgresql-x64-17" -D "C:\PostgreSQL\17\data" -w`), + so the loop normally costs one connect. ## Technical Details - `Options.RestartService string`; empty means no restart. From 44708f0728c3a1ac77c359c89b909518953ed918 Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 15:49:00 +0300 Subject: [PATCH 13/20] docs(ci): describe the refusal and start-pending runs --- CLAUDE.md | 15 +++++++++------ docs/architecture/provisioning.md | 17 ++++++++++------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b63758f..c7cf929 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -82,14 +82,17 @@ golangci-lint is pinned to 2.12.2 (`winget install GolangCI.golangci-lint --vers config in `.golangci.yml`. CI (`.github/workflows/ci.yml`) runs `go build`, `go test -race`, and the same lint on `windows-latest` and `ubuntu-latest` for every push and pull request. The Windows job then runs the `Restart the runner's PostgreSQL service through site` step, the -only test of the service restart: it sets the runner's Stopped and Disabled `postgresql-x64-17` -to Manual and starts it, requires the first `site --restart-service postgresql-x64-17` run to -print `restarting service postgresql-x64-17` and leave `listen_addresses = localhost` with no -setting pending, and requires a second run not to restart the service. +only test of the service restart. It requires `site --restart-service postgresql-x64-17` to +refuse the runner's shipped Disabled service with no `ALTER SYSTEM SET` line, sets it to Manual +and starts it with `sc.exe start`, which returns while it is Start Pending. It then requires the +next run, started at once, to print `restarting service postgresql-x64-17` and leave +`listen_addresses = localhost` with no setting pending, a second run not to restart the service, +and `--restart-service no-such-service` to be refused with no `ALTER SYSTEM SET` line. The Linux job provisions a `postgres:17-alpine` service container by running `site` twice — the tuning path and the idempotency check in one step. The -`Require the listen_addresses delta` step then requires `listen_addresses` still pending: the -container is never restarted, so a pending value proves `site` wrote the delta. +`Require the listen_addresses delta` step then requires `listen_addresses` still pending and +`localhost` in `postgresql.auto.conf` (`pg_file_settings`): the container is never restarted, so +the pending value and the file line prove `site` wrote the delta. The job then runs `bench` against a `postgres:14-alpine` container — the version floor, and the only place `REVOKE CREATE ON SCHEMA public FROM PUBLIC` bites, because 15 and later withhold that grant by engine default — provisions a third container with `bench` as a `postgres` image init script diff --git a/docs/architecture/provisioning.md b/docs/architecture/provisioning.md index 977ca0d..36ae72f 100644 --- a/docs/architecture/provisioning.md +++ b/docs/architecture/provisioning.md @@ -255,13 +255,16 @@ A restart drops every open connection to the server, the SCADA's included. CI's Linux job runs `site` without the flag: its server is a GitHub Actions service container, not a Windows service, and nothing in the job restarts it. The `Require the listen_addresses delta` -step then reads `listen_addresses` still pending, which proves `site` wrote the delta. The Windows -job's `Restart the runner's PostgreSQL service through site` step is the one test of the restart -itself. The runner image ships PostgreSQL 17 as `postgresql-x64-17`, Stopped and Disabled, so the -step sets it to Manual and starts it. It then requires the first -`site --restart-service postgresql-x64-17` run to print `restarting service postgresql-x64-17` -and to leave `listen_addresses = localhost` with no setting pending, and a second run not to -restart the service. +step then reads `listen_addresses` still pending and `localhost` in `postgresql.auto.conf` +through `pg_file_settings`, which proves `site` wrote the delta. The Windows job's +`Restart the runner's PostgreSQL service through site` step is the one test of the restart +itself. The runner image ships PostgreSQL 17 as `postgresql-x64-17`, Stopped and Disabled. The +step first requires `site --restart-service postgresql-x64-17` to refuse the Disabled service +with no `ALTER SYSTEM SET` line in its output. It then sets the service to Manual, starts it with +`sc.exe start`, and runs `site` at once, while the service is Start Pending. That run must print +`restarting service postgresql-x64-17` and leave `listen_addresses = localhost` with no setting +pending. A second run must not restart the service, and `--restart-service no-such-service` must +be refused with no `ALTER SYSTEM SET` line. ## The roles From 0e468d366b320fe4802ca5bd8d773baf13f0c11f Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:11:19 +0300 Subject: [PATCH 14/20] fix(provision): start only after the old server exits --- .github/workflows/ci.yml | 7 +- internal/provision/config.go | 5 +- internal/provision/service_other.go | 2 +- internal/provision/service_other_test.go | 2 +- internal/provision/service_windows.go | 125 ++++++++++++++++--- internal/provision/service_windows_test.go | 134 ++++++++++++++++++--- 6 files changed, 239 insertions(+), 36 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9852691..5753261 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,7 +34,8 @@ jobs: # while the service is still start pending, so the restart run begins # with openService waiting for Running, the way an installer that has # just started the service runs site; no other readiness wait precedes - # it. That run finds settings pending and restarts the service; the next + # it. That run finds settings pending and restarts the service, sending + # the Start only once the old server's port refuses; the next # finds nothing pending and must not restart it; a service that does not # exist is refused before any change. - name: Restart the runner's PostgreSQL service through site @@ -80,6 +81,10 @@ jobs: Write-Host "::error::the first site run did not restart $service" exit 1 } + if ($first.Output -notlike "*service $service stopped, nothing answers on localhost:5432*") { + Write-Host "::error::the first site run did not see the old server's port refused before the Start" + exit 1 + } $listen = & $psql -h 127.0.0.1 -U postgres -d postgres -tAc 'SHOW listen_addresses' if ($LASTEXITCODE -ne 0) { exit 1 } diff --git a/internal/provision/config.go b/internal/provision/config.go index 37ad566..6995ee1 100644 --- a/internal/provision/config.go +++ b/internal/provision/config.go @@ -4,7 +4,9 @@ import ( "context" "errors" "fmt" + "net" "slices" + "strconv" "strings" "time" @@ -47,7 +49,8 @@ func (o Options) config(ctx context.Context) error { if o.RestartService != "" { var err error // SA4023: off Windows openService always fails, by design - if service, err = openService(ctx, o.RestartService); err != nil { //nolint:staticcheck + address := net.JoinHostPort(o.Host, strconv.Itoa(o.Port)) + if service, err = openService(ctx, o.RestartService, address); err != nil { //nolint:staticcheck return err } defer service.close() diff --git a/internal/provision/service_other.go b/internal/provision/service_other.go index b949fc1..15c3507 100644 --- a/internal/provision/service_other.go +++ b/internal/provision/service_other.go @@ -11,7 +11,7 @@ import ( type managedService struct{} -func openService(_ context.Context, name string) (*managedService, error) { +func openService(_ context.Context, name, _ string) (*managedService, error) { return nil, fmt.Errorf("--restart-service %s: service restart is not supported on %s", name, runtime.GOOS) } diff --git a/internal/provision/service_other_test.go b/internal/provision/service_other_test.go index c698176..e271584 100644 --- a/internal/provision/service_other_test.go +++ b/internal/provision/service_other_test.go @@ -10,7 +10,7 @@ import ( ) func TestOpenServiceIsRefusedOffWindows(t *testing.T) { - service, err := openService(context.Background(), "postgresql-x64-17") + service, err := openService(context.Background(), "postgresql-x64-17", "localhost:5432") if err == nil { service.close() t.Fatal("openService() succeeded off Windows") diff --git a/internal/provision/service_windows.go b/internal/provision/service_windows.go index aaf7973..d610103 100644 --- a/internal/provision/service_windows.go +++ b/internal/provision/service_windows.go @@ -6,6 +6,7 @@ import ( "context" "errors" "fmt" + "net" "time" "golang.org/x/sys/windows" @@ -25,22 +26,36 @@ type serviceTimings struct { stop time.Duration start time.Duration poll time.Duration + // each accepted probe connection costs the server a process + probe time.Duration } func defaultServiceTimings() serviceTimings { - return serviceTimings{stop: 3 * time.Minute, start: 2 * time.Minute, poll: 500 * time.Millisecond} + return serviceTimings{ + stop: 3 * time.Minute, + start: 2 * time.Minute, + poll: 500 * time.Millisecond, + probe: time.Second, + } } +const listenerDialTimeout = 2 * time.Second + type managedService struct { name string control serviceControl - timings serviceTimings - release func() + // address is the server's host:port, which listening dials + address string + // listening reports whether a TCP connect to address succeeds; false with a nil error means + // the connect was refused, so no postmaster listens there + listening func(context.Context) (bool, error) + timings serviceTimings + release func() } // openService refuses a service the restart could not complete: missing, out of reach, // Disabled, or not running once a start in progress has finished. -func openService(ctx context.Context, name string) (*managedService, error) { +func openService(ctx context.Context, name, address string) (*managedService, error) { manager, err := mgr.Connect() if err != nil { if errors.Is(err, windows.ERROR_ACCESS_DENIED) { @@ -61,9 +76,11 @@ func openService(ctx context.Context, name string) (*managedService, error) { return nil, fmt.Errorf("opening service %s: %w", name, err) } service := &managedService{ - name: name, - control: handle, - timings: defaultServiceTimings(), + name: name, + control: handle, + address: address, + listening: dialListener(address), + timings: defaultServiceTimings(), release: func() { _ = handle.Close() _ = manager.Disconnect() @@ -92,6 +109,21 @@ func refuseDisabled(handle *mgr.Service, name string) error { return nil } +func dialListener(address string) func(context.Context) (bool, error) { + return func(ctx context.Context) (bool, error) { + dialer := net.Dialer{Timeout: listenerDialTimeout} + conn, err := dialer.DialContext(ctx, "tcp", address) + switch { + case err == nil: + _ = conn.Close() + return true, nil + case errors.Is(err, windows.WSAECONNREFUSED): + return false, nil + } + return false, err + } +} + // awaitRunning lets a start in progress finish: an installer may run site right after it started // the service, and an EDB service reports Running only once the server accepts connections. func (s *managedService) awaitRunning(ctx context.Context) error { @@ -114,7 +146,7 @@ func (s *managedService) close() { s.release() } -// restart detaches from the command's cancellation once the Stop is accepted: the Stopped wait, +// restart detaches from the command's cancellation once the Stop is accepted: the exit wait, // the Start and the Running wait each run to their own bound, so an interrupt or the command // timeout does not leave the service stopped. Residual cases: docs/architecture/provisioning.md. func (s *managedService) restart(ctx context.Context) error { @@ -122,10 +154,10 @@ func (s *managedService) restart(ctx context.Context) error { return fmt.Errorf("stopping service %s: %w", s.name, err) } detached := context.WithoutCancel(ctx) - if err := s.awaitStopped(detached); err != nil { + if err := s.awaitServerExit(detached); err != nil { return err } - ok("service %s stopped", s.name) + ok("service %s stopped, nothing answers on %s", s.name, s.address) if err := s.control.Start(); err != nil { return fmt.Errorf("starting service %s: %w; it stays stopped: start it with Start-Service %s", s.name, err, s.name) @@ -140,10 +172,20 @@ func (s *managedService) restart(ctx context.Context) error { return nil } -func (s *managedService) awaitStopped(ctx context.Context) error { +// awaitServerExit gives Stopped and the closed port one bound from the Stop: pg_ctl reports +// Stopped after at most a minute even while the postmaster still shuts down, and a Start then +// would meet the old postmaster's lock file. +func (s *managedService) awaitServerExit(ctx context.Context) error { waitCtx, cancel := context.WithTimeout(ctx, s.timings.stop) defer cancel() - state, err := s.pollUntil(waitCtx, func(state svc.State) bool { return state == svc.Stopped }) + if err := s.awaitStopped(waitCtx); err != nil { + return err + } + return s.awaitListenerClosed(waitCtx) +} + +func (s *managedService) awaitStopped(ctx context.Context) error { + state, err := s.pollUntil(ctx, func(state svc.State) bool { return state == svc.Stopped }) switch { case errors.Is(err, context.DeadlineExceeded): return fmt.Errorf("service %s is still %s %s after the Stop and will stay stopped; once "+ @@ -156,8 +198,39 @@ func (s *managedService) awaitStopped(ctx context.Context) error { return nil } -// awaitStarted fails at once on Stopped from the second query on: a postmaster that cannot start -// on the new settings exits, and the service falls back to Stopped. +func (s *managedService) awaitListenerClosed(ctx context.Context) error { + answered := false + var lastErr error + for { + answers, err := s.listening(ctx) + if !answers && err == nil { + return nil + } + // a dial cut short by the deadline says nothing about the server + if ctx.Err() == nil { + answered, lastErr = answers, err + } + select { + case <-ctx.Done(): + return s.serverOutlivedStop(answered, lastErr, ctx.Err()) + case <-time.After(s.timings.probe): + } + } +} + +func (s *managedService) serverOutlivedStop(answered bool, lastErr, cause error) error { + observed := "the server still answers on " + s.address + if !answered { + observed = fmt.Sprintf("connecting to %s still fails without a refusal (%v)", s.address, lastErr) + } + return fmt.Errorf("service %s is stopped, but %s %s after the Stop: the old server may still be "+ + "shutting down, so site sends no Start and the service stays stopped. Once a connection to "+ + "%s is refused, start it with Start-Service %s: %w", + s.name, observed, s.timings.stop, s.address, s.name, cause) +} + +// awaitStarted fails at once on Stopped from the second query on: pg_ctl reports Stopped when the +// postmaster exits during its startup or when its own startup wait runs out. func (s *managedService) awaitStarted(ctx context.Context) error { waitCtx, cancel := context.WithTimeout(ctx, s.timings.start) defer cancel() @@ -173,13 +246,31 @@ func (s *managedService) awaitStarted(ctx context.Context) error { case err != nil: return err case state == svc.Stopped: - return fmt.Errorf("service %s stopped again after the Start: the server did not start on the "+ - "new settings, which stay in postgresql.auto.conf. Read the server log in the data "+ - "directory's log folder and the Windows Application event log", s.name) + return s.stoppedAfterStart(ctx) } return nil } +func (s *managedService) stoppedAfterStart(ctx context.Context) error { + answers, err := s.listening(ctx) + switch { + case answers: + return fmt.Errorf("service %s stopped again after the Start, but the server answers on %s: "+ + "pg_ctl ran out of its startup wait (60 s unless the service's PathName sets -t) and "+ + "left the server running outside the service. Once it accepts connections, stop it "+ + "with pg_ctl stop -D , then start it with Start-Service %s", + s.name, s.address, s.name) + case err != nil: + return fmt.Errorf("service %s stopped again after the Start, and checking %s failed: %w. "+ + "Read the server log in the data directory's log folder and the Windows Application "+ + "event log", s.name, s.address, err) + } + return fmt.Errorf("service %s stopped again after the Start, and nothing answers on %s: the "+ + "server exited during its startup. Read the server log in the data directory's log folder "+ + "and the Windows Application event log; the settings this run wrote stay in "+ + "postgresql.auto.conf", s.name, s.address) +} + // pollUntil queries the service until done accepts its state or ctx ends, and returns the last // state it saw. func (s *managedService) pollUntil(ctx context.Context, done func(svc.State) bool) (svc.State, error) { diff --git a/internal/provision/service_windows_test.go b/internal/provision/service_windows_test.go index 394d669..1a9b046 100644 --- a/internal/provision/service_windows_test.go +++ b/internal/provision/service_windows_test.go @@ -5,6 +5,7 @@ package provision import ( "context" "errors" + "net" "strings" "testing" "time" @@ -12,14 +13,30 @@ import ( "golang.org/x/sys/windows/svc" ) -// fakeService answers Query with states in order and repeats the last one. +type probeResult struct { + answers bool + err error +} + +var ( + answering = probeResult{answers: true} + refusing = probeResult{} +) + +// fakeService answers Query with states and the listener probe with probes, each in order and +// repeating its last entry; no probes means a refused connect. type fakeService struct { states []svc.State + probes []probeResult queries int + probeCalls int controlErr error startErr error stops int starts int + // probe calls made before the Start, and whether the last of them saw the port refused + probesAtStart int + startAfterRefusal bool } func (f *fakeService) Query() (svc.Status, error) { @@ -35,14 +52,34 @@ func (f *fakeService) Control(svc.Cmd) (svc.Status, error) { func (f *fakeService) Start(...string) error { f.starts++ + f.probesAtStart = f.probeCalls + f.startAfterRefusal = f.probeCalls > 0 && f.lastProbe() == refusing return f.startErr } +func (f *fakeService) listening(context.Context) (bool, error) { + f.probeCalls++ + result := f.lastProbe() + return result.answers, result.err +} + +func (f *fakeService) lastProbe() probeResult { + if len(f.probes) == 0 { + return refusing + } + return f.probes[min(f.probeCalls, len(f.probes))-1] +} + func newFakeManagedService(fake *fakeService) *managedService { return &managedService{ - name: "pg", - control: fake, - timings: serviceTimings{stop: 50 * time.Millisecond, start: 50 * time.Millisecond, poll: time.Millisecond}, + name: "pg", + control: fake, + address: "localhost:5432", + listening: fake.listening, + timings: serviceTimings{ + stop: 50 * time.Millisecond, start: 50 * time.Millisecond, + poll: time.Millisecond, probe: time.Millisecond, + }, release: func() {}, } } @@ -63,14 +100,17 @@ var fullRestart = []svc.State{svc.StopPending, svc.StopPending, svc.Stopped, svc func TestRestart(t *testing.T) { tests := []struct { - name string - ctx context.Context - states []svc.State - controlErr error - startErr error - wantStarts int - wantErr []string - wantCause error + name string + ctx context.Context + states []svc.State + probes []probeResult + controlErr error + startErr error + wantStarts int + wantProbesAtStart int + wantErr []string + wantNotErr string + wantCause error }{ {name: "stops and starts", ctx: context.Background(), states: fullRestart, wantStarts: 1}, { @@ -92,15 +132,44 @@ func TestRestart(t *testing.T) { states: []svc.State{svc.StopPending}, wantErr: []string{"will stay stopped", "Start-Service pg"}, wantCause: context.DeadlineExceeded, }, + { + name: "Stopped while the server still answers: Start waits for the refusal", ctx: context.Background(), + states: fullRestart, probes: []probeResult{answering, answering, refusing}, + wantStarts: 1, wantProbesAtStart: 3, + }, + { + name: "a probe error is retried until the refusal", ctx: context.Background(), + states: fullRestart, probes: []probeResult{{err: errors.New("i/o timeout")}, refusing}, + wantStarts: 1, wantProbesAtStart: 2, + }, + { + name: "a server that outlives the stop bound gets no Start", ctx: context.Background(), + states: []svc.State{svc.Stopped}, probes: []probeResult{answering}, + wantErr: []string{"still answers on localhost:5432", "sends no Start", "Start-Service pg"}, + wantCause: context.DeadlineExceeded, + }, + { + name: "a probe that never gets a refusal gets no Start", ctx: context.Background(), + states: []svc.State{svc.Stopped}, probes: []probeResult{{err: errors.New("i/o timeout")}}, + wantErr: []string{"without a refusal (i/o timeout)", "Start-Service pg"}, + wantCause: context.DeadlineExceeded, + }, { name: "a refused Start says how to start it", ctx: context.Background(), states: []svc.State{svc.Stopped}, startErr: errors.New("start refused"), wantStarts: 1, wantErr: []string{"stays stopped", "Start-Service pg"}, }, { - name: "a server that falls back to Stopped fails at once", ctx: context.Background(), + name: "a server that exits during its startup fails at once", ctx: context.Background(), states: []svc.State{svc.Stopped, svc.StartPending, svc.Stopped}, wantStarts: 1, - wantErr: []string{"stopped again after the Start", "postgresql.auto.conf"}, + wantErr: []string{"stopped again after the Start", "nothing answers", "postgresql.auto.conf"}, + }, + { + name: "a startup pg_ctl stopped waiting for is not blamed on the settings", ctx: context.Background(), + states: []svc.State{svc.Stopped, svc.StartPending, svc.Stopped}, + probes: []probeResult{refusing, answering}, wantStarts: 1, + wantErr: []string{"stopped again after the Start", "outside the service", "pg_ctl stop -D"}, + wantNotErr: "postgresql.auto.conf", }, { name: "Stopped right after the Start is given one more query", ctx: context.Background(), @@ -114,7 +183,9 @@ func TestRestart(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fake := &fakeService{states: tt.states, controlErr: tt.controlErr, startErr: tt.startErr} + fake := &fakeService{ + states: tt.states, probes: tt.probes, controlErr: tt.controlErr, startErr: tt.startErr, + } err := newFakeManagedService(fake).restart(tt.ctx) if fake.stops != 1 { t.Errorf("Stop sent %d times, want 1", fake.stops) @@ -122,7 +193,16 @@ func TestRestart(t *testing.T) { if fake.starts != tt.wantStarts { t.Errorf("Start sent %d times, want %d", fake.starts, tt.wantStarts) } + if fake.starts > 0 && !fake.startAfterRefusal { + t.Errorf("Start sent after %d probes, the last not a refusal", fake.probesAtStart) + } + if tt.wantProbesAtStart > 0 && fake.probesAtStart != tt.wantProbesAtStart { + t.Errorf("Start sent after %d probes, want %d", fake.probesAtStart, tt.wantProbesAtStart) + } assertServiceError(t, err, tt.wantErr, tt.wantCause) + if tt.wantNotErr != "" && err != nil && strings.Contains(err.Error(), tt.wantNotErr) { + t.Errorf("error = %q, want it not to contain %q", err, tt.wantNotErr) + } }) } } @@ -160,6 +240,30 @@ func TestAwaitRunning(t *testing.T) { } } +// localhost resolves to ::1 first on Windows, so the listener on 127.0.0.1 also proves that a +// refusal on one address does not hide an answer on the other +func TestDialListener(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + _, port, err := net.SplitHostPort(listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + probe := dialListener(net.JoinHostPort("localhost", port)) + + if answers, err := probe(context.Background()); !answers || err != nil { + t.Errorf("with a listener: answers = %v, error = %v, want true, nil", answers, err) + } + if err := listener.Close(); err != nil { + t.Fatal(err) + } + if answers, err := probe(context.Background()); answers || err != nil { + t.Errorf("after the listener closed: answers = %v, error = %v, want false, nil", answers, err) + } +} + func assertServiceError(t *testing.T, err error, wantText []string, wantCause error) { t.Helper() if wantText == nil { From ba6e8bb756d23984bec431d8989c21059c6ce78b Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:11:19 +0300 Subject: [PATCH 15/20] docs(site): name the exit wait and start timeout --- CLAUDE.md | 9 ++-- docs/architecture/provisioning.md | 44 ++++++++++++++----- docs/deployment.md | 15 +++++-- .../20261019-site-loopback-and-restart.md | 34 +++++++++----- 4 files changed, 75 insertions(+), 27 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c7cf929..8fd6605 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -85,8 +85,9 @@ The Windows job then runs the `Restart the runner's PostgreSQL service through s only test of the service restart. It requires `site --restart-service postgresql-x64-17` to refuse the runner's shipped Disabled service with no `ALTER SYSTEM SET` line, sets it to Manual and starts it with `sc.exe start`, which returns while it is Start Pending. It then requires the -next run, started at once, to print `restarting service postgresql-x64-17` and leave -`listen_addresses = localhost` with no setting pending, a second run not to restart the service, +next run, started at once, to print `restarting service postgresql-x64-17` and +`stopped, nothing answers on localhost:5432` and leave `listen_addresses = localhost` with no +setting pending, a second run not to restart the service, and `--restart-service no-such-service` to be refused with no `ALTER SYSTEM SET` line. The Linux job provisions a `postgres:17-alpine` service container by running `site` twice — the tuning path and the idempotency check in one step. The @@ -153,7 +154,9 @@ restart. Without a flag the run warns and the operator restarts. `site --restart restarts that Windows service inside the tuning phase when settings wait, so `create` and the tail check run against the restarted server. Only `site` registers the flag, and it refuses a `--host` other than `localhost`, `127.0.0.1` or `::1`; the service is checked before anything -changes, and once it accepts the Stop the restart runs to completion despite an interrupt +changes, the Start waits until the old server's port refuses connections, since `pg_ctl` +reports Stopped before a slow postmaster exits, and once the service accepts the Stop the restart +runs to completion despite an interrupt (`docs/architecture/provisioning.md#the-service-restart`, which also lists the cases that still leave it stopped). diff --git a/docs/architecture/provisioning.md b/docs/architecture/provisioning.md index 36ae72f..5824224 100644 --- a/docs/architecture/provisioning.md +++ b/docs/architecture/provisioning.md @@ -181,8 +181,8 @@ The package that runs the SQL owns its guards; the CLI is a thin dispatcher. in-flight query through pgx instead of killing the process mid-DDL. The context itself — `signal.NotifyContext(..., os.Interrupt)` plus the five-minute per-command timeout (`phaseTimeout`) — is owned by the CLI dispatcher in `cmd/semibase`. The one exception is a - service restart: once the service accepts the Stop, the waits and the Start ignore the - command's cancellation, so that an interrupt does not leave the service stopped + service restart: once the service accepts the Stop, the waits, the port probe and the Start + ignore the command's cancellation, so that an interrupt does not leave the service stopped ([The service restart](#the-service-restart)). A run that used up `phaseTimeout` says so in a second error line. - **Pending-restart reporting.** The tuning phase applies its deltas with `pg_reload_conf()` and @@ -222,25 +222,49 @@ The restart runs inside `config`, right after `pendingRestartSettings` returns a `create` and `check` run against the restarted server. `applySettings` closes the phase connection before it returns; then: -1. Stop, then wait up to 3 minutes from the Stop for Stopped: a shutdown checkpoint with - `max_wal_size = 8GB` can be long. +1. Stop, then wait for Stopped and then for a refused TCP connect to `--host:--port`, both within + one bound of 3 minutes from the Stop. If the bound runs out, the Start is not sent. 2. Start, then wait up to 2 minutes from the Start for Running. Stopped after the first query fails - at once: the postmaster exited on the new settings. + at once, and one more connect to `--host:--port` names the cause (see below). 3. Connect to `postgres` as the superuser every second, up to 60 s. A refused connection and 57P03 are retried; any other server error, a wrong password for one, fails at once. A timeout reports the last connect error, never the context's. 4. Read `pending_restart` on the new connection. A non-empty list is an error naming the settings. +Stopped does not mean that the postmaster has exited. What follows was read in +`src/bin/pg_ctl/pg_ctl.c` and `src/backend/postmaster/postmaster.c` of PostgreSQL REL_17_STABLE: + +- On a Stop, `pgwin32_ServiceMain` sends the postmaster SIGINT (fast shutdown) and waits for its + process at most 12 times 5 s. Then it reports Stopped and exits, whether the postmaster is gone + or not. The job object it puts the postmaster in has no `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE`, so + the postmaster outlives it and finishes its shutdown checkpoint, which with + `max_wal_size = 8GB` can take longer than that minute. +- A Start in that window launches a postmaster that finds the old one's live PID in + `postmaster.pid` and exits FATAL "lock file already exists" (`CreateLockFile` in `miscinit.c`). +- The postmaster keeps its listening sockets until its exit callbacks close them, just before + they remove `postmaster.pid`. A shutting-down postmaster that no longer accepts still completes + the TCP handshake through the listen backlog. So a refused connect (`WSAECONNREFUSED`) means the + postmaster is gone, and step 1 waits for it. Any other connect error is retried. +- Step 1 probes once a second, because each accepted connect costs the server a process. The + probe dials the address the run connects to. `Validate` already restricts it to loopback, so it + needs nothing the run does not have. A `postmaster.pid` check would need the service's `-D`. +- The service runs `pg_ctl runservice ... -w` with no `-t`, so `wait_for_postmaster_start` gives + up after `DEFAULT_WAIT` = 60 s. It then reports Stopped and leaves a slower postmaster running + outside the service. A postmaster that exits during its startup also ends in Stopped. In step 2 + the probe tells the two apart: an answer means the timed-out startup, a refusal means the exit. + Only a hung `pg_ctl` reaches the 2-minute bound. + Once the service accepts the Stop, steps 1 and 2 run on `context.WithoutCancel`, each to its own bound. An interrupt or the command timeout during them does not stop the sequence: the service is -started, and the run then ends with the cancellation. Each wait error names the service and the -last state it observed. Three cases still leave the service stopped: +started, and the run then ends with the cancellation. A wait that runs out its bound names the +service and the last state it observed. Four cases still leave the service stopped: | Case | What the run says | Recovery | | --- | --- | --- | -| The stop takes longer than 3 minutes | the service is still stopping and will stay stopped | once `Get-Service ` shows Stopped, `Start-Service ` | +| The old postmaster still runs 3 minutes after the Stop, or `pg_ctl` itself has not reported Stopped by then | the service is stopped but the server still answers, or the service is still stopping; no Start was sent and the service stays stopped | once a connect to the port is refused (`Test-NetConnection localhost -Port `) and `Get-Service ` shows Stopped, `Start-Service ` | | The process ends without running its code: console window closed, logoff, shutdown, a killed process. `signal.NotifyContext` catches `os.Interrupt` only, and Windows ends a console process a few seconds after a close event, which no handler can extend | nothing | `Start-Service ` | -| The postmaster refuses the new settings, for example it cannot allocate `shared_buffers = 2GB`, and the service falls back to Stopped | the service stopped again after the Start; the settings stay in `postgresql.auto.conf` | read the server log in the data directory's `log` folder, remove the offending line from `postgresql.auto.conf`, then `Start-Service ` | +| The postmaster exits during its startup, for example it cannot allocate `shared_buffers = 2GB` | the service stopped again after the Start and nothing answers on the port; the settings this run wrote stay in `postgresql.auto.conf` | read the server log in the data directory's `log` folder, remove the offending line from `postgresql.auto.conf`, then `Start-Service ` | +| The startup takes longer than `pg_ctl`'s 60 s wait | the service stopped again after the Start but the server answers on the port: it runs outside the service | once it accepts connections, `pg_ctl stop -D `, then `Start-Service ` | The command's five-minute `phaseTimeout` also covers the restart. A stop and a start that use their whole bounds leave no time for the ready wait, `create` and `check`; the run then fails with @@ -262,7 +286,7 @@ itself. The runner image ships PostgreSQL 17 as `postgresql-x64-17`, Stopped and step first requires `site --restart-service postgresql-x64-17` to refuse the Disabled service with no `ALTER SYSTEM SET` line in its output. It then sets the service to Manual, starts it with `sc.exe start`, and runs `site` at once, while the service is Start Pending. That run must print -`restarting service postgresql-x64-17` and leave `listen_addresses = localhost` with no setting +`restarting service postgresql-x64-17` and `stopped, nothing answers on localhost:5432`, and leave `listen_addresses = localhost` with no setting pending. A second run must not restart the service, and `--restart-service no-such-service` must be refused with no `ALTER SYSTEM SET` line. diff --git a/docs/deployment.md b/docs/deployment.md index a17fa97..775e984 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -189,9 +189,18 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па примет подключения, и продолжает работу. Без флага перезапустите службу или перезагрузите машину; пока перезапуск не сделан, утилита предупреждает об этом в последних строках вывода. - Если утилита сообщает, что служба остановлена и такой останется, запустите её командой - `Start-Service postgresql-x64-17`. Если служба остановилась сразу после запуска, сервер не - принял новые настройки: причина записана в журнале сервера, в папке `log` каталога данных. + Утилита запускает службу, только когда старый сервер завершил работу и порт перестал + отвечать. Если она сообщает, что служба остановлена и такой останется, дождитесь, пока + `Test-NetConnection localhost -Port 5432` покажет `TcpTestSucceeded : False`, и запустите + службу командой `Start-Service postgresql-x64-17`. + + Если служба остановилась сразу после запуска, утилита сообщает, отвечает ли сервер на порту. + Если ничего не отвечает, сервер завершился при старте: причина записана в журнале сервера, в + папке `log` каталога данных, а записанные утилитой настройки остаются в `postgresql.auto.conf`. + Если сервер отвечает, служба не дождалась конца его старта, и сервер работает вне службы. Когда + он начнёт принимать подключения, остановите его командой `pg_ctl stop -D <каталог данных>` + (`pg_ctl.exe` лежит в папке `bin` установки PostgreSQL, каталог данных указан после `-D` в пути + к исполняемому файлу службы) и запустите службу командой `Start-Service postgresql-x64-17`. Перезапуск разрывает подключения SCADA. Повторный запуск `site` на работающей установке перезапускает службу, только если какие-то настройки ждут перезапуска. Их список выводит diff --git a/docs/plans/20261019-site-loopback-and-restart.md b/docs/plans/20261019-site-loopback-and-restart.md index 2cf44be..42404af 100644 --- a/docs/plans/20261019-site-loopback-and-restart.md +++ b/docs/plans/20261019-site-loopback-and-restart.md @@ -81,13 +81,21 @@ (`service_other.go` returns "service restart is not supported on ") fails with nothing changed. The module stays one cross-platform build: the Linux binary is what the docker benches run. -- Once the service accepts the Stop, the Stopped wait, the Start and the Running wait run on a +- Once the service accepts the Stop, the exit wait, the Start and the Running wait run on a context detached from the command's cancellation and timeout (`context.WithoutCancel`), each wait with its own bound. An interrupt or the command timeout during them does not stop the - sequence. Three cases still leave the service stopped, each with its recovery in - `docs/architecture/provisioning.md#the-service-restart`: a stop longer than its bound, a process - ended by the console close, logoff, shutdown or a kill, and a postmaster that refuses the new - settings. + sequence. +- SCM Stopped does not prove the postmaster is gone: `pg_ctl runservice` reports Stopped after at + most 12 x 5 s of a stop and leaves a slower postmaster running (`pg_ctl.c`, REL_17_STABLE). So + the Start waits, within the stop bound, for a TCP connect to `--host:--port` to be refused; the + run already knows that address, while a `postmaster.pid` check would need the service's `-D`. +- `pg_ctl -w` without `-t` gives up on a startup after 60 s and reports Stopped while the + postmaster keeps starting. Stopped after the Start therefore probes the port once: an answer + names that timeout, a refusal names a postmaster that exited during its startup. +- Four cases still leave the service stopped, each with its recovery in + `docs/architecture/provisioning.md#the-service-restart`: an old postmaster still running at the + stop bound, a process ended by the console close, logoff, shutdown or a kill, a postmaster that + exits during its startup, and a startup longer than `pg_ctl`'s 60 s wait. - After the service reports Running, the tool reconnects in a bounded loop. An EDB service runs `pg_ctl runservice ... -w` and reports Running only once the postmaster accepts connections (read with CIM on the development machine, and printed by the Windows CI step for @@ -108,19 +116,23 @@ 1. Close the phase connection. 2. `Stop` on the command context. From here on, steps 3 and 4 run on `context.WithoutCancel(ctx)`. - 3. Wait for `Stopped`, bounded by 3 minutes from the Stop: a shutdown checkpoint with - `max_wal_size = 8GB` can be long. If the wait gives up, Start is not sent and the error says - the service will stay stopped and how to start it. + 3. Wait for `Stopped`, then for a refused TCP connect to `--host:--port` (`WSAECONNREFUSED`; + any other connect error is retried), probing once a second, both bounded by 3 minutes from + the Stop: a shutdown checkpoint with `max_wal_size = 8GB` can be long. If the wait gives up, + Start is not sent and the error says the service will stay stopped and how to start it. 4. `Start`, then wait for `Running`, bounded by 2 minutes from the Start. Stopped from the - second query on fails at once and points to the server log. + second query on fails at once; one more probe decides the message: an answer means + `pg_ctl`'s 60 s startup wait ran out and the server runs outside the service, a refusal + means the server exited during its startup and points to the server log. 5. The ready wait connects to `postgres` as the superuser every second, bounded by 60 s. It retries network errors and 57P03 only, returns the new connection, and the last connect error that is not a context error goes into the timeout message. 6. `pendingRestartSettings` runs on the new connection. A non-empty list is an error naming the settings, with a hint that the service may belong to another server. - Service control goes through a three-method seam over `*mgr.Service` (`Query`, `Control`, - `Start`), so the sequence is unit-tested with scripted states. -- The wait errors name the service and its last observed state. + `Start`), and the port probe is a function field on `managedService`, so the sequence is + unit-tested with scripted states and probe results. +- A wait that runs out its bound names the service and its last observed state. - Output: `step("restarting service %s: %s waiting for a restart", name, list)`, then `ok` lines for stopped, running, accepting connections. From 3671802bf56cbb6e1bba1e2ad932ea1b2a31a164 Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:14:39 +0300 Subject: [PATCH 16/20] docs(site): separate kernel and postmaster facts --- docs/architecture/provisioning.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/architecture/provisioning.md b/docs/architecture/provisioning.md index 5824224..a82b6a0 100644 --- a/docs/architecture/provisioning.md +++ b/docs/architecture/provisioning.md @@ -242,8 +242,9 @@ Stopped does not mean that the postmaster has exited. What follows was read in - A Start in that window launches a postmaster that finds the old one's live PID in `postmaster.pid` and exits FATAL "lock file already exists" (`CreateLockFile` in `miscinit.c`). - The postmaster keeps its listening sockets until its exit callbacks close them, just before - they remove `postmaster.pid`. A shutting-down postmaster that no longer accepts still completes - the TCP handshake through the listen backlog. So a refused connect (`WSAECONNREFUSED`) means the + they remove `postmaster.pid` (`CloseServerPorts`). In `PM_WAIT_DEAD_END` it stops accepting + but keeps the sockets open, and the kernel still completes a TCP handshake into the listen + backlog. So a refused connect (`WSAECONNREFUSED`) means the postmaster is gone, and step 1 waits for it. Any other connect error is retried. - Step 1 probes once a second, because each accepted connect costs the server a process. The probe dials the address the run connects to. `Validate` already restricts it to loopback, so it From 9c17a8944d0841b44c362b2c01674741ad98a0d3 Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:38:03 +0300 Subject: [PATCH 17/20] fix(provision): tie the service to the run's server --- .github/workflows/ci.yml | 12 +- CLAUDE.md | 12 +- docs/architecture/provisioning.md | 46 ++-- docs/deployment.md | 20 +- .../20261019-site-loopback-and-restart.md | 34 ++- internal/provision/config.go | 34 ++- internal/provision/service_other.go | 6 +- internal/provision/service_other_test.go | 6 +- internal/provision/service_windows.go | 106 +++++++-- internal/provision/service_windows_test.go | 203 +++++++++++++++--- 10 files changed, 377 insertions(+), 102 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5753261..4032e40 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,8 +34,9 @@ jobs: # while the service is still start pending, so the restart run begins # with openService waiting for Running, the way an installer that has # just started the service runs site; no other readiness wait precedes - # it. That run finds settings pending and restarts the service, sending - # the Start only once the old server's port refuses; the next + # it. That run ties the service to the server on port 5432 through the + # -D in its PathName, finds settings pending and restarts the service, + # sending the Start only once the old server's address refuses; the next # finds nothing pending and must not restart it; a service that does not # exist is refused before any change. - name: Restart the runner's PostgreSQL service through site @@ -81,11 +82,16 @@ jobs: Write-Host "::error::the first site run did not restart $service" exit 1 } - if ($first.Output -notlike "*service $service stopped, nothing answers on localhost:5432*") { + if ($first.Output -notlike "*service $service runs data directory C:\PostgreSQL\17\data, as the server on *:5432 does*") { + Write-Host "::error::the first site run did not tie $service to the server on port 5432" + exit 1 + } + if ($first.Output -notlike "*service $service stopped, nothing answers on *:5432*") { Write-Host "::error::the first site run did not see the old server's port refused before the Start" exit 1 } + Write-Host "data_directory: $(& $psql -h 127.0.0.1 -U postgres -d postgres -tAc 'SHOW data_directory')" $listen = & $psql -h 127.0.0.1 -U postgres -d postgres -tAc 'SHOW listen_addresses' if ($LASTEXITCODE -ne 0) { exit 1 } $pending = & $psql -h 127.0.0.1 -U postgres -d postgres -tAc ` diff --git a/CLAUDE.md b/CLAUDE.md index 8fd6605..16ad8e9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -85,8 +85,9 @@ The Windows job then runs the `Restart the runner's PostgreSQL service through s only test of the service restart. It requires `site --restart-service postgresql-x64-17` to refuse the runner's shipped Disabled service with no `ALTER SYSTEM SET` line, sets it to Manual and starts it with `sc.exe start`, which returns while it is Start Pending. It then requires the -next run, started at once, to print `restarting service postgresql-x64-17` and -`stopped, nothing answers on localhost:5432` and leave `listen_addresses = localhost` with no +next run, started at once, to print `restarting service postgresql-x64-17`, the line tying the +service's `-D` to the server on port 5432, and `stopped, nothing answers on :5432`, and leave +`listen_addresses = localhost` with no setting pending, a second run not to restart the service, and `--restart-service no-such-service` to be refused with no `ALTER SYSTEM SET` line. The Linux job provisions a `postgres:17-alpine` service container by running `site` @@ -153,9 +154,10 @@ Two of the settings, `shared_buffers` and `listen_addresses = 'localhost'`, wait restart. Without a flag the run warns and the operator restarts. `site --restart-service ` restarts that Windows service inside the tuning phase when settings wait, so `create` and the tail check run against the restarted server. Only `site` registers the flag, and it refuses a -`--host` other than `localhost`, `127.0.0.1` or `::1`; the service is checked before anything -changes, the Start waits until the old server's port refuses connections, since `pg_ctl` -reports Stopped before a slow postmaster exits, and once the service accepts the Stop the restart +`--host` other than `localhost`, `127.0.0.1` or `::1`. The service is checked before anything +changes, including that the `-D` in its `PathName` is the `data_directory` of the server the run +connects to. The Start waits until the IP:port that connection reached refuses connections, since +`pg_ctl` reports Stopped before a slow postmaster exits, and once the service accepts the Stop the restart runs to completion despite an interrupt (`docs/architecture/provisioning.md#the-service-restart`, which also lists the cases that still leave it stopped). diff --git a/docs/architecture/provisioning.md b/docs/architecture/provisioning.md index a82b6a0..9ee1f6b 100644 --- a/docs/architecture/provisioning.md +++ b/docs/architecture/provisioning.md @@ -204,28 +204,36 @@ restart, and `bench --restart-service` fails as an undefined flag. `Options.Vali `--host` other than `localhost`, `127.0.0.1` or `::1`, because after the restart the server listens on the loopback interface only. -`config` opens the service before it connects, so a service it cannot restart fails the run with -nothing changed. `openService` (`service_windows.go`) refuses a service that does not exist, a +`config` opens the service before `applySettings` connects, so a service it cannot restart, or one +that is not the run's server, fails the run with nothing changed. `openService` (`service_windows.go`) refuses a service that does not exist, a service manager or service the run may not control (the message says to run `semibase` as an administrator), and a Disabled start type, which would leave the service stopped after the Stop. A service in Start Pending gets up to 2 minutes to reach Running: an installer may run `site` right after it started the service, and an EDB service (`pg_ctl runservice ... -w`, the `PathName` the Windows CI step prints) reports Running only once the server accepts connections. That step starts the service with `sc.exe start`, which returns while it is Start Pending, and runs `site` at once. Stopped, Stop Pending, Paused, or a start that ends in Stopped, -is refused. The tool does not check that the service is the server behind `--port`: a wrong name -restarts another running instance, and the run then fails with the settings still pending and a -hint to compare the service's `-D` with `SHOW data_directory`. Off Windows, `service_other.go` -refuses the flag the same way and names the platform, so the module stays one cross-platform -build. +is refused. + +Once the service is Running, `openService` ties it to the server the run connects to. It connects +as the superuser, reads `SHOW data_directory` and the connection's remote IP:port, and compares +the directory with the `-D` argument of the service's `PathName`, split with +`windows.DecomposeCommandLine`. `pg_ctl register` writes that argument quoted, absolute and with +backslashes; the server reports forward slashes. The comparison runs `filepath.Clean` on both +paths and ignores case. A mismatch, or a `PathName` with no `-D`, is refused with nothing +changed: a Stop sent to another service would leave that service stopped, because the run's own +server would keep answering the probe. Without a `-D`, `pg_ctl` takes the data directory from the +service's environment, which the tool does not read. Off Windows, `service_other.go` refuses the +flag the same way and names the platform, so the module stays one cross-platform build. The restart runs inside `config`, right after `pendingRestartSettings` returns a non-empty list, so `create` and `check` run against the restarted server. `applySettings` closes the phase connection before it returns; then: -1. Stop, then wait for Stopped and then for a refused TCP connect to `--host:--port`, both within - one bound of 3 minutes from the Stop. If the bound runs out, the Start is not sent. +1. Stop, then wait for Stopped and then for a refused TCP connect to the IP:port the identity + check's connection reached, both within one bound of 3 minutes from the Stop. If the bound runs + out, the Start is not sent. 2. Start, then wait up to 2 minutes from the Start for Running. Stopped after the first query fails - at once, and one more connect to `--host:--port` names the cause (see below). + at once, and one more connect to that address names the cause (see below). 3. Connect to `postgres` as the superuser every second, up to 60 s. A refused connection and 57P03 are retried; any other server error, a wrong password for one, fails at once. A timeout reports the last connect error, never the context's. @@ -247,8 +255,16 @@ Stopped does not mean that the postmaster has exited. What follows was read in backlog. So a refused connect (`WSAECONNREFUSED`) means the postmaster is gone, and step 1 waits for it. Any other connect error is retried. - Step 1 probes once a second, because each accepted connect costs the server a process. The - probe dials the address the run connects to. `Validate` already restricts it to loopback, so it - needs nothing the run does not have. A `postmaster.pid` check would need the service's `-D`. + probe dials the one literal IP:port the identity check's connection reached, never the name + `localhost`. Go dials a name that resolves to both `::1` and `127.0.0.1` in parallel and, when + both fail, returns the error of the first family (`dialParallel` in `net/dial.go`, Go 1.27). + An IPv6 failure other than a refusal would then hide the IPv4 refusal, and the Start would never + be sent. A literal address has one family. +- Each probe dial has a 15 s timeout. Windows resends a SYN after an RST, so a refused loopback + connect can take seconds (Daniel Stenberg, + `daniel.haxx.se/blog/2024/08/14/slow-tcp-connect-on-windows`). Measured on Windows 11 build + 26300, a refusal took under 2 ms; the resends were not reproduced here. The 3-minute stop bound + still limits the whole wait. - The service runs `pg_ctl runservice ... -w` with no `-t`, so `wait_for_postmaster_start` gives up after `DEFAULT_WAIT` = 60 s. It then reports Stopped and leaves a slower postmaster running outside the service. A postmaster that exits during its startup also ends in Stopped. In step 2 @@ -264,7 +280,7 @@ service and the last state it observed. Four cases still leave the service stopp | --- | --- | --- | | The old postmaster still runs 3 minutes after the Stop, or `pg_ctl` itself has not reported Stopped by then | the service is stopped but the server still answers, or the service is still stopping; no Start was sent and the service stays stopped | once a connect to the port is refused (`Test-NetConnection localhost -Port `) and `Get-Service ` shows Stopped, `Start-Service ` | | The process ends without running its code: console window closed, logoff, shutdown, a killed process. `signal.NotifyContext` catches `os.Interrupt` only, and Windows ends a console process a few seconds after a close event, which no handler can extend | nothing | `Start-Service ` | -| The postmaster exits during its startup, for example it cannot allocate `shared_buffers = 2GB` | the service stopped again after the Start and nothing answers on the port; the settings this run wrote stay in `postgresql.auto.conf` | read the server log in the data directory's `log` folder, remove the offending line from `postgresql.auto.conf`, then `Start-Service ` | +| The postmaster exits during its startup, for example it cannot allocate `shared_buffers = 2GB` | the service stopped again after the Start and nothing answers on the port; the message names the recovery | read the server log in the data directory's `log` folder, remove the line it names from `postgresql.auto.conf`, then `Start-Service `. A re-run of `site` writes the line again and stops the service the same way, so fix the cause first | | The startup takes longer than `pg_ctl`'s 60 s wait | the service stopped again after the Start but the server answers on the port: it runs outside the service | once it accepts connections, `pg_ctl stop -D `, then `Start-Service ` | The command's five-minute `phaseTimeout` also covers the restart. A stop and a start that use @@ -287,7 +303,9 @@ itself. The runner image ships PostgreSQL 17 as `postgresql-x64-17`, Stopped and step first requires `site --restart-service postgresql-x64-17` to refuse the Disabled service with no `ALTER SYSTEM SET` line in its output. It then sets the service to Manual, starts it with `sc.exe start`, and runs `site` at once, while the service is Start Pending. That run must print -`restarting service postgresql-x64-17` and `stopped, nothing answers on localhost:5432`, and leave `listen_addresses = localhost` with no setting +`restarting service postgresql-x64-17`, the identity line +`runs data directory C:\PostgreSQL\17\data, as the server on :5432 does` and +`stopped, nothing answers on :5432`, and leave `listen_addresses = localhost` with no setting pending. A second run must not restart the service, and `--restart-service no-such-service` must be refused with no `ALTER SYSTEM SET` line. diff --git a/docs/deployment.md b/docs/deployment.md index 775e984..ee161b9 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -174,7 +174,9 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па ``` `postgresql-x64-17` — имя службы, которую создаёт установщик PostgreSQL 17. Проверить его можно -командой `Get-Service postgresql*`. +командой `Get-Service postgresql*`. Перед любыми изменениями утилита сверяет каталог после `-D` в +пути к исполняемому файлу службы с каталогом данных сервера на порту. Если они не совпадают, флаг +называет не ту службу, и утилита завершается с ошибкой, ничего не изменив. На выходе утилита гарантирует: настройки сервера применены, архивная база, три роли, права и права по умолчанию созданы, пять таблиц SemiPlot, функция `semiplot_register_new_pens()` и @@ -195,13 +197,21 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па службу командой `Start-Service postgresql-x64-17`. Если служба остановилась сразу после запуска, утилита сообщает, отвечает ли сервер на порту. - Если ничего не отвечает, сервер завершился при старте: причина записана в журнале сервера, в - папке `log` каталога данных, а записанные утилитой настройки остаются в `postgresql.auto.conf`. - Если сервер отвечает, служба не дождалась конца его старта, и сервер работает вне службы. Когда + Если ничего не отвечает, сервер завершился при старте. Причина записана в журнале сервера, в + папке `log` каталога данных. Если журнал называет настройку, которую записала утилита + (например, серверу не хватило памяти на `shared_buffers = 2GB`), удалите эту строку из + `postgresql.auto.conf` в каталоге данных и запустите службу командой + `Start-Service postgresql-x64-17`. Повторный запуск `site` запишет строку снова и так же + остановит службу, поэтому сначала устраните причину. Если сервер отвечает, служба не дождалась конца его старта, и сервер работает вне службы. Когда он начнёт принимать подключения, остановите его командой `pg_ctl stop -D <каталог данных>` (`pg_ctl.exe` лежит в папке `bin` установки PostgreSQL, каталог данных указан после `-D` в пути к исполняемому файлу службы) и запустите службу командой `Start-Service postgresql-x64-17`. + Если во время перезапуска закрыли окно PowerShell, вышли из системы или завершили процесс + утилиты, служба может остаться остановленной. Проверьте её командой + `Get-Service postgresql-x64-17` и, если она остановлена, запустите командой + `Start-Service postgresql-x64-17`. + Перезапуск разрывает подключения SCADA. Повторный запуск `site` на работающей установке перезапускает службу, только если какие-то настройки ждут перезапуска. Их список выводит запрос `SELECT name FROM pg_settings WHERE pending_restart`; если он не пуст, остановите SCADA @@ -240,6 +250,6 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па | `--port` | `5432` | Порт сервера | | `--database` | `scada_archive` | Имя архивной базы | | `--expected-major` | не проверяется | Требуемая мажорная версия сервера | -| `--restart-service` | не задан | Только `site` в Windows: служба PostgreSQL, которую утилита перезапускает, если настройки ждут перезапуска. Требует `--host` `localhost`, `127.0.0.1` или `::1` и запуска от имени администратора. Отключённую или остановленную службу утилита не трогает и завершается с ошибкой; только что запущенную службу ждёт до двух минут | +| `--restart-service` | не задан | Только `site` в Windows: служба PostgreSQL, которую утилита перезапускает, если настройки ждут перезапуска. Требует `--host` `localhost`, `127.0.0.1` или `::1` и запуска от имени администратора. Отключённую, остановленную или обслуживающую другой каталог данных службу утилита не трогает и завершается с ошибкой; только что запущенную службу ждёт до двух минут | Полный перечень — `semibase <команда> --help`. diff --git a/docs/plans/20261019-site-loopback-and-restart.md b/docs/plans/20261019-site-loopback-and-restart.md index 42404af..182eeb1 100644 --- a/docs/plans/20261019-site-loopback-and-restart.md +++ b/docs/plans/20261019-site-loopback-and-restart.md @@ -81,14 +81,22 @@ (`service_other.go` returns "service restart is not supported on ") fails with nothing changed. The module stays one cross-platform build: the Linux binary is what the docker benches run. +- The service must be the run's server. Once it is Running, `openService` connects, reads + `SHOW data_directory` and the connection's remote IP:port, and compares the directory with the + `-D` in the service's `PathName` (`windows.DecomposeCommandLine`, `filepath.Clean` on both + sides, case ignored). A mismatch or a `PathName` with no `-D` fails with nothing changed: a Stop + sent to another service would leave it stopped while the run's server keeps answering. - Once the service accepts the Stop, the exit wait, the Start and the Running wait run on a context detached from the command's cancellation and timeout (`context.WithoutCancel`), each wait with its own bound. An interrupt or the command timeout during them does not stop the sequence. - SCM Stopped does not prove the postmaster is gone: `pg_ctl runservice` reports Stopped after at most 12 x 5 s of a stop and leaves a slower postmaster running (`pg_ctl.c`, REL_17_STABLE). So - the Start waits, within the stop bound, for a TCP connect to `--host:--port` to be refused; the - run already knows that address, while a `postmaster.pid` check would need the service's `-D`. + the Start waits, within the stop bound, for a TCP connect to be refused. The probe dials the + literal IP:port the identity check's connection reached: Go dials `localhost` to `::1` and + `127.0.0.1` in parallel and reports the first family's error, so a non-refusal on one family + would hide the refusal on the other. Each dial has a 15 s timeout, since Windows resends a SYN + after an RST. - `pg_ctl -w` without `-t` gives up on a startup after 60 s and reports Stopped while the postmaster keeps starting. Stopped after the Start therefore probes the port once: an answer names that timeout, a refusal names a postmaster that exited during its startup. @@ -111,27 +119,33 @@ - The restart runs when the pending list is non-empty and a service was opened; the condition sits inline in `config`'s switch. - `openService` waits up to 2 minutes for a Start Pending service to reach Running, then refuses - any state other than Running. + any state other than Running. It then calls `describeServer` (`config.go`), which connects as + the superuser and returns `SHOW data_directory` and `RemoteAddr()`, and refuses a service whose + `-D` differs or is absent. The address becomes the probe's target. - Restart sequence in `config`: 1. Close the phase connection. 2. `Stop` on the command context. From here on, steps 3 and 4 run on `context.WithoutCancel(ctx)`. - 3. Wait for `Stopped`, then for a refused TCP connect to `--host:--port` (`WSAECONNREFUSED`; - any other connect error is retried), probing once a second, both bounded by 3 minutes from + 3. Wait for `Stopped`, then for a refused TCP connect to the identity check's IP:port + (`WSAECONNREFUSED`; any other connect error is retried; each dial times out after 15 s), + probing once a second, both bounded by 3 minutes from the Stop: a shutdown checkpoint with `max_wal_size = 8GB` can be long. If the wait gives up, Start is not sent and the error says the service will stay stopped and how to start it. 4. `Start`, then wait for `Running`, bounded by 2 minutes from the Start. Stopped from the second query on fails at once; one more probe decides the message: an answer means `pg_ctl`'s 60 s startup wait ran out and the server runs outside the service, a refusal - means the server exited during its startup and points to the server log. + means the server exited during its startup and points to the server log, to removing the + line it names from `postgresql.auto.conf` before `Start-Service`, and warns that a re-run + writes the line again. 5. The ready wait connects to `postgres` as the superuser every second, bounded by 60 s. It retries network errors and 57P03 only, returns the new connection, and the last connect error that is not a context error goes into the timeout message. 6. `pendingRestartSettings` runs on the new connection. A non-empty list is an error naming the - settings, with a hint that the service may belong to another server. -- Service control goes through a three-method seam over `*mgr.Service` (`Query`, `Control`, - `Start`), and the port probe is a function field on `managedService`, so the sequence is - unit-tested with scripted states and probe results. + settings. +- Service control goes through a four-method seam over `*mgr.Service` (`Query`, `Control`, + `Start`, `Config`), the port probe is a function field on `managedService`, and the server read + is a function `openService` receives, so the check and the sequence are unit-tested with + scripted states, configurations, servers and probe results. - A wait that runs out its bound names the service and its last observed state. - Output: `step("restarting service %s: %s waiting for a restart", name, list)`, then `ok` lines for stopped, running, accepting connections. diff --git a/internal/provision/config.go b/internal/provision/config.go index 6995ee1..a96202c 100644 --- a/internal/provision/config.go +++ b/internal/provision/config.go @@ -4,9 +4,7 @@ import ( "context" "errors" "fmt" - "net" "slices" - "strconv" "strings" "time" @@ -49,8 +47,7 @@ func (o Options) config(ctx context.Context) error { if o.RestartService != "" { var err error // SA4023: off Windows openService always fails, by design - address := net.JoinHostPort(o.Host, strconv.Itoa(o.Port)) - if service, err = openService(ctx, o.RestartService, address); err != nil { //nolint:staticcheck + if service, err = openService(ctx, o.RestartService, o.describeServer); err != nil { //nolint:staticcheck return err } defer service.close() @@ -71,6 +68,29 @@ func (o Options) config(ctx context.Context) error { return nil } +// runningServer is what a connection to --host:--port learns about the server behind it. +type runningServer struct { + dataDirectory string + // address is the IP:port the connection reached, one address of one family + address string +} + +func (o Options) describeServer(ctx context.Context) (runningServer, error) { + conn, err := o.connectSuperuser(ctx, "postgres") + if err != nil { + return runningServer{}, err + } + defer conn.Close(ctx) + var dataDirectory string + if err := conn.QueryRow(ctx, "SHOW data_directory").Scan(&dataDirectory); err != nil { + return runningServer{}, fmt.Errorf("reading data_directory: %w", err) + } + return runningServer{ + dataDirectory: dataDirectory, + address: conn.PgConn().Conn().RemoteAddr().String(), + }, nil +} + // applySettings closes its connection before returning: a restart must not find it open. func (o Options) applySettings(ctx context.Context) ([]string, error) { conn, err := o.connectSuperuser(ctx, "postgres") @@ -128,10 +148,8 @@ func (o Options) restartService(ctx context.Context, service *managedService, pe return err } if len(stillPending) > 0 { - return fmt.Errorf("after restarting service %s, settings still wait for a restart: %s. "+ - "The service may belong to another server than the one on port %d: compare the -D in "+ - "its PathName (Get-CimInstance Win32_Service) with SHOW data_directory", - o.RestartService, strings.Join(stillPending, ", "), o.Port) + return fmt.Errorf("after restarting service %s, settings still wait for a restart: %s", + o.RestartService, strings.Join(stillPending, ", ")) } ok("all settings active") return nil diff --git a/internal/provision/service_other.go b/internal/provision/service_other.go index 15c3507..7f9cff4 100644 --- a/internal/provision/service_other.go +++ b/internal/provision/service_other.go @@ -11,7 +11,11 @@ import ( type managedService struct{} -func openService(_ context.Context, name, _ string) (*managedService, error) { +func openService( + _ context.Context, + name string, + _ func(context.Context) (runningServer, error), +) (*managedService, error) { return nil, fmt.Errorf("--restart-service %s: service restart is not supported on %s", name, runtime.GOOS) } diff --git a/internal/provision/service_other_test.go b/internal/provision/service_other_test.go index e271584..50800ad 100644 --- a/internal/provision/service_other_test.go +++ b/internal/provision/service_other_test.go @@ -10,7 +10,11 @@ import ( ) func TestOpenServiceIsRefusedOffWindows(t *testing.T) { - service, err := openService(context.Background(), "postgresql-x64-17", "localhost:5432") + describe := func(context.Context) (runningServer, error) { + t.Fatal("openService() described the server off Windows") + return runningServer{}, nil + } + service, err := openService(context.Background(), "postgresql-x64-17", describe) if err == nil { service.close() t.Fatal("openService() succeeded off Windows") diff --git a/internal/provision/service_windows.go b/internal/provision/service_windows.go index d610103..0f97c3b 100644 --- a/internal/provision/service_windows.go +++ b/internal/provision/service_windows.go @@ -7,6 +7,8 @@ import ( "errors" "fmt" "net" + "path/filepath" + "strings" "time" "golang.org/x/sys/windows" @@ -19,6 +21,7 @@ type serviceControl interface { Query() (svc.Status, error) Control(c svc.Cmd) (svc.Status, error) Start(args ...string) error + Config() (mgr.Config, error) } type serviceTimings struct { @@ -39,12 +42,13 @@ func defaultServiceTimings() serviceTimings { } } -const listenerDialTimeout = 2 * time.Second +// Windows resends a SYN after an RST, so even a refused loopback connect can take seconds +const listenerDialTimeout = 15 * time.Second type managedService struct { name string control serviceControl - // address is the server's host:port, which listening dials + // address is the IP:port the run's connection reached, which listening dials address string // listening reports whether a TCP connect to address succeeds; false with a nil error means // the connect was refused, so no postmaster listens there @@ -53,9 +57,14 @@ type managedService struct { release func() } -// openService refuses a service the restart could not complete: missing, out of reach, -// Disabled, or not running once a start in progress has finished. -func openService(ctx context.Context, name, address string) (*managedService, error) { +// openService refuses a service the restart could not complete or that is not the server the run +// connects to: missing, out of reach, Disabled, not running once a start in progress has +// finished, or registered for another data directory than the one describe reads. +func openService( + ctx context.Context, + name string, + describe func(context.Context) (runningServer, error), +) (*managedService, error) { manager, err := mgr.Connect() if err != nil { if errors.Is(err, windows.ERROR_ACCESS_DENIED) { @@ -76,39 +85,92 @@ func openService(ctx context.Context, name, address string) (*managedService, er return nil, fmt.Errorf("opening service %s: %w", name, err) } service := &managedService{ - name: name, - control: handle, - address: address, - listening: dialListener(address), - timings: defaultServiceTimings(), + name: name, + control: handle, + timings: defaultServiceTimings(), release: func() { _ = handle.Close() _ = manager.Disconnect() }, } - if err := refuseDisabled(handle, name); err != nil { - service.close() - return nil, err - } - if err := service.awaitRunning(ctx); err != nil { + if err := service.check(ctx, describe); err != nil { service.close() return nil, err } + service.listening = dialListener(service.address) return service, nil } -func refuseDisabled(handle *mgr.Service, name string) error { - config, err := handle.Config() +func (s *managedService) check( + ctx context.Context, + describe func(context.Context) (runningServer, error), +) error { + config, err := s.control.Config() if err != nil { - return fmt.Errorf("reading the configuration of service %s: %w", name, err) + return fmt.Errorf("reading the configuration of service %s: %w", s.name, err) } if config.StartType == mgr.StartDisabled { return fmt.Errorf("service %s is Disabled, so it could not be started again after the stop; "+ - "set its start type to Automatic or Manual", name) + "set its start type to Automatic or Manual", s.name) + } + if err := s.awaitRunning(ctx); err != nil { + return err + } + return s.confirmServes(ctx, config.BinaryPathName, describe) +} + +// confirmServes ties the service to the server behind --host:--port through the data directory: +// stopping another service would leave it stopped, since the probe would keep finding the +// run's server answering. +func (s *managedService) confirmServes( + ctx context.Context, + pathName string, + describe func(context.Context) (runningServer, error), +) error { + serviceDirectory, found := dataDirectoryFlag(pathName) + if !found { + return fmt.Errorf("the PathName of service %s carries no -D, so site cannot tell which data "+ + "directory it runs; site restarts a service registered with pg_ctl register -D", s.name) + } + server, err := describe(ctx) + if err != nil { + return err } + if !sameDataDirectory(serviceDirectory, server.dataDirectory) { + return fmt.Errorf("service %s runs data directory %s, but the server on %s uses %s: "+ + "--restart-service names another server. Find the service whose PathName carries "+ + "-D %s with Get-CimInstance Win32_Service", s.name, serviceDirectory, server.address, + server.dataDirectory, filepath.Clean(server.dataDirectory)) + } + ok("service %s runs data directory %s, as the server on %s does", s.name, serviceDirectory, + server.address) + s.address = server.address return nil } +// dataDirectoryFlag reads the -D argument of a service command line, as pg_ctl register writes it. +func dataDirectoryFlag(pathName string) (string, bool) { + args, err := windows.DecomposeCommandLine(pathName) + if err != nil { + return "", false + } + for i, arg := range args { + switch { + case arg == "-D" && i+1 < len(args): + return args[i+1], true + case strings.HasPrefix(arg, "-D") && len(arg) > len("-D"): + return strings.TrimPrefix(arg, "-D"), true + } + } + return "", false +} + +// sameDataDirectory compares the service's native path with the server's, which PostgreSQL +// reports with forward slashes. +func sameDataDirectory(servicePath, serverPath string) bool { + return strings.EqualFold(filepath.Clean(servicePath), filepath.Clean(serverPath)) +} + func dialListener(address string) func(context.Context) (bool, error) { return func(ctx context.Context) (bool, error) { dialer := net.Dialer{Timeout: listenerDialTimeout} @@ -267,8 +329,10 @@ func (s *managedService) stoppedAfterStart(ctx context.Context) error { } return fmt.Errorf("service %s stopped again after the Start, and nothing answers on %s: the "+ "server exited during its startup. Read the server log in the data directory's log folder "+ - "and the Windows Application event log; the settings this run wrote stay in "+ - "postgresql.auto.conf", s.name, s.address) + "and the Windows Application event log. If it names a setting this run wrote, remove that "+ + "line from postgresql.auto.conf, then start the service with Start-Service %s; a re-run "+ + "of site writes the line again and stops the service the same way until the cause is fixed", + s.name, s.address, s.name) } // pollUntil queries the service until done accepts its state or ctx ends, and returns the last diff --git a/internal/provision/service_windows_test.go b/internal/provision/service_windows_test.go index 1a9b046..a04623b 100644 --- a/internal/provision/service_windows_test.go +++ b/internal/provision/service_windows_test.go @@ -3,6 +3,7 @@ package provision import ( + "cmp" "context" "errors" "net" @@ -11,6 +12,7 @@ import ( "time" "golang.org/x/sys/windows/svc" + "golang.org/x/sys/windows/svc/mgr" ) type probeResult struct { @@ -26,6 +28,7 @@ var ( // fakeService answers Query with states and the listener probe with probes, each in order and // repeating its last entry; no probes means a refused connect. type fakeService struct { + config mgr.Config states []svc.State probes []probeResult queries int @@ -45,6 +48,10 @@ func (f *fakeService) Query() (svc.Status, error) { return svc.Status{State: state}, nil } +func (f *fakeService) Config() (mgr.Config, error) { + return f.config, nil +} + func (f *fakeService) Control(svc.Cmd) (svc.Status, error) { f.stops++ return svc.Status{State: svc.StopPending}, f.controlErr @@ -74,7 +81,7 @@ func newFakeManagedService(fake *fakeService) *managedService { return &managedService{ name: "pg", control: fake, - address: "localhost:5432", + address: "[::1]:5432", listening: fake.listening, timings: serviceTimings{ stop: 50 * time.Millisecond, start: 50 * time.Millisecond, @@ -145,7 +152,7 @@ func TestRestart(t *testing.T) { { name: "a server that outlives the stop bound gets no Start", ctx: context.Background(), states: []svc.State{svc.Stopped}, probes: []probeResult{answering}, - wantErr: []string{"still answers on localhost:5432", "sends no Start", "Start-Service pg"}, + wantErr: []string{"still answers on [::1]:5432", "sends no Start", "Start-Service pg"}, wantCause: context.DeadlineExceeded, }, { @@ -162,7 +169,10 @@ func TestRestart(t *testing.T) { { name: "a server that exits during its startup fails at once", ctx: context.Background(), states: []svc.State{svc.Stopped, svc.StartPending, svc.Stopped}, wantStarts: 1, - wantErr: []string{"stopped again after the Start", "nothing answers", "postgresql.auto.conf"}, + wantErr: []string{ + "stopped again after the Start", "nothing answers", "postgresql.auto.conf", + "Start-Service pg", "writes the line again", + }, }, { name: "a startup pg_ctl stopped waiting for is not blamed on the settings", ctx: context.Background(), @@ -207,32 +217,96 @@ func TestRestart(t *testing.T) { } } -func TestAwaitRunning(t *testing.T) { +const runnerPathName = `"C:\Program Files\PostgreSQL\17\bin\pg_ctl.exe" runservice -N "pg" -D "C:\PostgreSQL\17\data" -w` + +var runnerServer = runningServer{dataDirectory: "C:/PostgreSQL/17/data", address: "[::1]:5432"} + +func TestCheck(t *testing.T) { tests := []struct { - name string - states []svc.State - wantErr []string - wantCause error + name string + startType uint32 + pathName string + states []svc.State + server runningServer + describeErr error + wantErr []string + wantCause error + wantDescribe bool }{ - {name: "running", states: []svc.State{svc.Running}}, - {name: "a start in progress is waited out", states: []svc.State{svc.StartPending, svc.StartPending, svc.Running}}, { - name: "a start past its bound is refused", states: []svc.State{svc.StartPending}, - wantErr: []string{"still start pending"}, wantCause: context.DeadlineExceeded, + name: "the running service of the run's server", pathName: runnerPathName, + states: []svc.State{svc.Running}, server: runnerServer, wantDescribe: true, + }, + { + name: "a start in progress is waited out", pathName: runnerPathName, + states: []svc.State{svc.StartPending, svc.StartPending, svc.Running}, server: runnerServer, + wantDescribe: true, }, - {name: "stopped is refused", states: []svc.State{svc.Stopped}, wantErr: []string{"is stopped, not running"}}, - {name: "stopping is refused", states: []svc.State{svc.StopPending}, wantErr: []string{"is stop pending"}}, - {name: "paused is refused", states: []svc.State{svc.Paused}, wantErr: []string{"is paused"}}, { - name: "a start that fails is refused", states: []svc.State{svc.StartPending, svc.Stopped}, + name: "Disabled is refused", startType: mgr.StartDisabled, pathName: runnerPathName, + states: []svc.State{svc.Running}, wantErr: []string{"is Disabled"}, + }, + { + name: "a start past its bound is refused", pathName: runnerPathName, + states: []svc.State{svc.StartPending}, wantErr: []string{"still start pending"}, + wantCause: context.DeadlineExceeded, + }, + { + name: "stopped is refused", pathName: runnerPathName, states: []svc.State{svc.Stopped}, wantErr: []string{"is stopped, not running"}, }, + { + name: "stopping is refused", pathName: runnerPathName, states: []svc.State{svc.StopPending}, + wantErr: []string{"is stop pending"}, + }, + { + name: "paused is refused", pathName: runnerPathName, states: []svc.State{svc.Paused}, + wantErr: []string{"is paused"}, + }, + { + name: "a start that fails is refused", pathName: runnerPathName, + states: []svc.State{svc.StartPending, svc.Stopped}, wantErr: []string{"is stopped, not running"}, + }, + { + name: "a service with no -D is refused", pathName: `C:\Scada\scada.exe --service`, + states: []svc.State{svc.Running}, wantErr: []string{"carries no -D"}, + }, + { + name: "a service of another data directory is refused", pathName: runnerPathName, + states: []svc.State{svc.Running}, wantDescribe: true, + server: runningServer{dataDirectory: "C:/Program Files/PostgreSQL/14/data", address: "[::1]:5432"}, + wantErr: []string{ + `runs data directory C:\PostgreSQL\17\data`, "names another server", + `-D C:\Program Files\PostgreSQL\14\data`, + }, + }, + { + name: "a failed read of the server is returned", pathName: runnerPathName, + states: []svc.State{svc.Running}, describeErr: errors.New("connection refused"), + wantDescribe: true, wantErr: []string{"connection refused"}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fake := &fakeService{states: tt.states} - err := newFakeManagedService(fake).awaitRunning(context.Background()) + fake := &fakeService{ + config: mgr.Config{StartType: cmp.Or(tt.startType, mgr.StartManual), BinaryPathName: tt.pathName}, + states: tt.states, + } + described := false + describe := func(context.Context) (runningServer, error) { + described = true + return tt.server, tt.describeErr + } + service := newFakeManagedService(fake) + service.address = "" + err := service.check(context.Background(), describe) assertServiceError(t, err, tt.wantErr, tt.wantCause) + if described != tt.wantDescribe { + t.Errorf("described the server = %v, want %v", described, tt.wantDescribe) + } + if err == nil && service.address != tt.server.address { + t.Errorf("address = %q, want the connection's %q", service.address, tt.server.address) + } if fake.stops != 0 || fake.starts != 0 { t.Errorf("the check sent %d Stop and %d Start, want none", fake.stops, fake.starts) } @@ -240,28 +314,89 @@ func TestAwaitRunning(t *testing.T) { } } -// localhost resolves to ::1 first on Windows, so the listener on 127.0.0.1 also proves that a -// refusal on one address does not hide an answer on the other -func TestDialListener(t *testing.T) { - listener, err := net.Listen("tcp", "127.0.0.1:0") - if err != nil { - t.Fatal(err) +func TestDataDirectoryFlag(t *testing.T) { + tests := []struct { + name string + pathName string + want string + wantFound bool + }{ + { + name: "pg_ctl register, quoted with spaces", + pathName: `"C:\Program Files\PostgreSQL\14\bin\pg_ctl.exe" runservice -N "postgresql-14" -D "C:\Program Files\PostgreSQL\14\data" -w`, + want: `C:\Program Files\PostgreSQL\14\data`, wantFound: true, + }, + { + name: "unquoted", + pathName: `C:\pg\bin\pg_ctl.exe runservice -D C:\pg\data -w`, + want: `C:\pg\data`, wantFound: true, + }, + { + name: "value attached to the flag", + pathName: `C:\pg\bin\pg_ctl.exe runservice -DC:\pg\data`, + want: `C:\pg\data`, wantFound: true, + }, + {name: "no -D", pathName: `"C:\pg\bin\pg_ctl.exe" runservice -N "pg" -w`}, + {name: "-D with no value", pathName: `C:\pg\bin\pg_ctl.exe runservice -D`}, } - _, port, err := net.SplitHostPort(listener.Addr().String()) - if err != nil { - t.Fatal(err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, found := dataDirectoryFlag(tt.pathName) + if got != tt.want || found != tt.wantFound { + t.Errorf("dataDirectoryFlag() = %q, %v, want %q, %v", got, found, tt.want, tt.wantFound) + } + }) } - probe := dialListener(net.JoinHostPort("localhost", port)) +} - if answers, err := probe(context.Background()); !answers || err != nil { - t.Errorf("with a listener: answers = %v, error = %v, want true, nil", answers, err) +func TestSameDataDirectory(t *testing.T) { + const server = "C:/PostgreSQL/17/data" + tests := []struct { + name string + servicePath string + want bool + }{ + {name: "forward slashes on the server side", servicePath: `C:\PostgreSQL\17\data`, want: true}, + {name: "case", servicePath: `c:\postgresql\17\DATA`, want: true}, + {name: "trailing separator", servicePath: `C:\PostgreSQL\17\data\`, want: true}, + {name: "another directory", servicePath: `C:\Program Files\PostgreSQL\14\data`}, + {name: "a longer name with the same prefix", servicePath: `C:\PostgreSQL\17\data2`}, } - if err := listener.Close(); err != nil { - t.Fatal(err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := sameDataDirectory(tt.servicePath, server); got != tt.want { + t.Errorf("sameDataDirectory(%q, %q) = %v, want %v", tt.servicePath, server, got, tt.want) + } + }) } - if answers, err := probe(context.Background()); answers || err != nil { - t.Errorf("after the listener closed: answers = %v, error = %v, want false, nil", answers, err) +} + +// The probe dials one literal address, so the other loopback family cannot decide the answer. +func TestDialListener(t *testing.T) { + for _, host := range []string{"127.0.0.1", "::1"} { + t.Run(host+" answers, then refuses", func(t *testing.T) { + listener, err := net.Listen("tcp", net.JoinHostPort(host, "0")) + if err != nil { + t.Skipf("no %s listener on this machine: %v", host, err) + } + probe := dialListener(listener.Addr().String()) + if answers, err := probe(context.Background()); !answers || err != nil { + t.Errorf("with a listener: answers = %v, error = %v, want true, nil", answers, err) + } + if err := listener.Close(); err != nil { + t.Fatal(err) + } + if answers, err := probe(context.Background()); answers || err != nil { + t.Errorf("after the listener closed: answers = %v, error = %v, want false, nil", answers, err) + } + }) } + t.Run("a connect error other than a refusal is not taken as gone", func(t *testing.T) { + answers, err := dialListener("127.0.0.1:0")(context.Background()) + if answers || err == nil { + t.Errorf("answers = %v, error = %v, want false and an error", answers, err) + } + }) } func assertServiceError(t *testing.T, err error, wantText []string, wantCause error) { From 19a1d92d400ebd83ef2e00666132110c616924e5 Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:48:55 +0300 Subject: [PATCH 18/20] docs(site): align restart docs and tests with code --- CLAUDE.md | 23 ++++++++--------- cmd/semibase/main_test.go | 9 +++++-- docs/architecture/provisioning.md | 25 +++++++++++-------- docs/deployment.md | 19 ++++++++------ .../20261019-site-loopback-and-restart.md | 9 ++++--- internal/provision/config.go | 4 +-- internal/provision/service_windows.go | 3 ++- 7 files changed, 53 insertions(+), 39 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 16ad8e9..e89ecf8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,9 +8,9 @@ architecture docs. Deployment target: Windows. Language: Go (module `github.com/Semiteq/SemiBase`), driver `pgx/v5`, the five `sql/semiplot_*.sql` files and `sql/trends.sql` embedded via `go:embed`. Entry point: `cmd/semibase`. The module is pure Go (`pgx/v5`, plus `golang.org/x/sys` for the Windows service -restart) and compiles for any GOOS; Linux builds serve the -containerised test bench of consumers (SemiPlot runs `bench` against an ephemeral `postgres:17` -container). All commands run from the repository root. +restart) and compiles for any GOOS; Linux builds serve the containerised test bench of consumers +(SemiPlot runs `bench` against an ephemeral `postgres:17` container). All commands run from the +repository root. ## Build @@ -87,14 +87,13 @@ refuse the runner's shipped Disabled service with no `ALTER SYSTEM SET` line, se and starts it with `sc.exe start`, which returns while it is Start Pending. It then requires the next run, started at once, to print `restarting service postgresql-x64-17`, the line tying the service's `-D` to the server on port 5432, and `stopped, nothing answers on :5432`, and leave -`listen_addresses = localhost` with no -setting pending, a second run not to restart the service, +`listen_addresses = localhost` with no setting pending, a second run not to restart the service, and `--restart-service no-such-service` to be refused with no `ALTER SYSTEM SET` line. -The Linux job provisions a `postgres:17-alpine` service container by running `site` -twice — the tuning path and the idempotency check in one step. The -`Require the listen_addresses delta` step then requires `listen_addresses` still pending and -`localhost` in `postgresql.auto.conf` (`pg_file_settings`): the container is never restarted, so -the pending value and the file line prove `site` wrote the delta. +The Linux job provisions a `postgres:17-alpine` service container by running `site` twice — the +tuning path and the idempotency check in one step. The `Require the listen_addresses delta` step +then requires `listen_addresses` still pending and `localhost` in `postgresql.auto.conf` +(`pg_file_settings`): the container is never restarted, so the pending value and the file line +prove `site` wrote the delta. The job then runs `bench` against a `postgres:14-alpine` container — the version floor, and the only place `REVOKE CREATE ON SCHEMA public FROM PUBLIC` bites, because 15 and later withhold that grant by engine default — provisions a third container with `bench` as a `postgres` image init script @@ -157,8 +156,8 @@ tail check run against the restarted server. Only `site` registers the flag, and `--host` other than `localhost`, `127.0.0.1` or `::1`. The service is checked before anything changes, including that the `-D` in its `PathName` is the `data_directory` of the server the run connects to. The Start waits until the IP:port that connection reached refuses connections, since -`pg_ctl` reports Stopped before a slow postmaster exits, and once the service accepts the Stop the restart -runs to completion despite an interrupt +`pg_ctl` reports Stopped before a slow postmaster exits, and once the service accepts the Stop the +restart runs to completion despite an interrupt (`docs/architecture/provisioning.md#the-service-restart`, which also lists the cases that still leave it stopped). diff --git a/cmd/semibase/main_test.go b/cmd/semibase/main_test.go index 5851902..21baa47 100644 --- a/cmd/semibase/main_test.go +++ b/cmd/semibase/main_test.go @@ -94,7 +94,8 @@ func TestParseOptions(t *testing.T) { check func(t *testing.T, options provision.Options) }{ { - name: "no restart flag parses as before", + name: "site without --restart-service restarts nothing", + command: "site", check: func(t *testing.T, options provision.Options) { if options.RestartService != "" { t.Errorf("RestartService = %q, want empty", options.RestartService) @@ -182,7 +183,11 @@ func TestParseOptions(t *testing.T) { t.Setenv(name, value) } - options, err := parseOptions(tt.command, tt.arguments) + command := tt.command + if command == "" { + command = "bench" + } + options, err := parseOptions(command, tt.arguments) switch { case tt.wantErrText != "": diff --git a/docs/architecture/provisioning.md b/docs/architecture/provisioning.md index 9ee1f6b..2bb2a0b 100644 --- a/docs/architecture/provisioning.md +++ b/docs/architecture/provisioning.md @@ -205,14 +205,15 @@ restart, and `bench --restart-service` fails as an undefined flag. `Options.Vali on the loopback interface only. `config` opens the service before `applySettings` connects, so a service it cannot restart, or one -that is not the run's server, fails the run with nothing changed. `openService` (`service_windows.go`) refuses a service that does not exist, a -service manager or service the run may not control (the message says to run `semibase` as an -administrator), and a Disabled start type, which would leave the service stopped after the Stop. -A service in Start Pending gets up to 2 minutes to reach Running: an installer may run `site` right -after it started the service, and an EDB service (`pg_ctl runservice ... -w`, the `PathName` the -Windows CI step prints) reports Running only once the server accepts connections. That step starts -the service with `sc.exe start`, which returns while it is Start Pending, and runs `site` at once. Stopped, Stop Pending, Paused, or a start that ends in Stopped, -is refused. +that is not the run's server, fails the run with nothing changed. `openService` +(`service_windows.go`) refuses a service that does not exist, a service manager or service the run +may not control (the message says to run `semibase` as an administrator), and a Disabled start +type, which would leave the service stopped after the Stop. A service in Start Pending gets up to +2 minutes to reach Running: an installer may run `site` right after it started the service, and an +EDB service (`pg_ctl runservice ... -w`, the `PathName` the Windows CI step prints) reports Running +only once the server accepts connections. That step starts the service with `sc.exe start`, which +returns while it is Start Pending, and runs `site` at once. Stopped, Stop Pending, Paused, or a +start that ends in Stopped, is refused. Once the service is Running, `openService` ties it to the server the run connects to. It connects as the superuser, reads `SHOW data_directory` and the connection's remote IP:port, and compares @@ -234,9 +235,11 @@ before it returns; then: out, the Start is not sent. 2. Start, then wait up to 2 minutes from the Start for Running. Stopped after the first query fails at once, and one more connect to that address names the cause (see below). -3. Connect to `postgres` as the superuser every second, up to 60 s. A refused connection and 57P03 - are retried; any other server error, a wrong password for one, fails at once. A timeout - reports the last connect error, never the context's. +3. Connect to `postgres` as the superuser every second, up to 60 s. Every error that is not a + server error, a refused connection for one, is retried, and so is 57P03; any other server + error, a wrong password for one, fails at once. A timeout reports the last connect error the + deadline did not cut short; when no dial failed before the deadline, it reports the error of + the dial the deadline cut short. 4. Read `pending_restart` on the new connection. A non-empty list is an error naming the settings. Stopped does not mean that the postmaster has exited. What follows was read in diff --git a/docs/deployment.md b/docs/deployment.md index ee161b9..ec25351 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -193,8 +193,9 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па Утилита запускает службу, только когда старый сервер завершил работу и порт перестал отвечать. Если она сообщает, что служба остановлена и такой останется, дождитесь, пока - `Test-NetConnection localhost -Port 5432` покажет `TcpTestSucceeded : False`, и запустите - службу командой `Start-Service postgresql-x64-17`. + `Test-NetConnection localhost -Port 5432` покажет `TcpTestSucceeded : False` и + `Get-Service postgresql-x64-17` покажет `Stopped`, и запустите службу командой + `Start-Service postgresql-x64-17`. Если служба остановилась сразу после запуска, утилита сообщает, отвечает ли сервер на порту. Если ничего не отвечает, сервер завершился при старте. Причина записана в журнале сервера, в @@ -202,10 +203,11 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па (например, серверу не хватило памяти на `shared_buffers = 2GB`), удалите эту строку из `postgresql.auto.conf` в каталоге данных и запустите службу командой `Start-Service postgresql-x64-17`. Повторный запуск `site` запишет строку снова и так же - остановит службу, поэтому сначала устраните причину. Если сервер отвечает, служба не дождалась конца его старта, и сервер работает вне службы. Когда - он начнёт принимать подключения, остановите его командой `pg_ctl stop -D <каталог данных>` - (`pg_ctl.exe` лежит в папке `bin` установки PostgreSQL, каталог данных указан после `-D` в пути - к исполняемому файлу службы) и запустите службу командой `Start-Service postgresql-x64-17`. + остановит службу, поэтому сначала устраните причину. Если сервер отвечает, служба не + дождалась конца его старта, и сервер работает вне службы. Когда он начнёт принимать + подключения, остановите его командой `pg_ctl stop -D <каталог данных>` (`pg_ctl.exe` лежит в + папке `bin` установки PostgreSQL, каталог данных указан после `-D` в пути к исполняемому файлу + службы) и запустите службу командой `Start-Service postgresql-x64-17`. Если во время перезапуска закрыли окно PowerShell, вышли из системы или завершили процесс утилиты, служба может остаться остановленной. Проверьте её командой @@ -218,8 +220,9 @@ $env:SEMIBASE_PLOT_PASSWORD = [Net.NetworkCredential]::new('', (Read-Host 'Па перед запуском. 5. Запустить проект Simple-Scada против базы. Проект подключают к `localhost` или `127.0.0.1`: - после перезапуска сервер принимает подключения только с этой машины. Мы рассчитываем, что таблицу `trends` он застанет - готовой, оставит её как есть и дальше создаст дневные партиции и `messages` сам. **Это + после перезапуска сервер принимает подключения только с этой машины. Мы рассчитываем, что + таблицу `trends` он застанет готовой, оставит её как есть и дальше создаст дневные партиции и + `messages` сам. **Это предположение не проверено на реальном проекте.** На первом запуске включите `log_statement = 'all'` и прочитайте в журнале PostgreSQL, какие DDL-команды SCADA выдала: если она создаёт или изменяет `trends`, порядок ввода в эксплуатацию нужно diff --git a/docs/plans/20261019-site-loopback-and-restart.md b/docs/plans/20261019-site-loopback-and-restart.md index 182eeb1..f4fb990 100644 --- a/docs/plans/20261019-site-loopback-and-restart.md +++ b/docs/plans/20261019-site-loopback-and-restart.md @@ -52,7 +52,10 @@ so the value stays pending; this proves the delta is written. - Windows CI, new step against the runner's PostgreSQL 17, with `$psql = "$env:PGBIN\psql.exe"`: 1. `semibase site --port 5432 --expected-major 17 --restart-service postgresql-x64-17` exits 0, - and its output contains `restarting service postgresql-x64-17`. + and its output contains `restarting service postgresql-x64-17`, the identity line + `service postgresql-x64-17 runs data directory C:\PostgreSQL\17\data, as the server on + :5432 does` and `service postgresql-x64-17 stopped, nothing answers on :5432`, where + `` is the literal address the identity check's connection reached. 2. `& $psql -h 127.0.0.1 -U postgres -d postgres -tAc "SHOW listen_addresses"` prints `localhost`, and `SELECT count(*) FROM pg_settings WHERE pending_restart` prints `0`. 3. A second identical run exits 0, and its output does not contain `restarting service`. @@ -138,8 +141,8 @@ line it names from `postgresql.auto.conf` before `Start-Service`, and warns that a re-run writes the line again. 5. The ready wait connects to `postgres` as the superuser every second, bounded by 60 s. It - retries network errors and 57P03 only, returns the new connection, and the last connect - error that is not a context error goes into the timeout message. + retries every error that is not a server error, and 57P03, returns the new connection, and + the last connect error the deadline did not cut short goes into the timeout message. 6. `pendingRestartSettings` runs on the new connection. A non-empty list is an error naming the settings. - Service control goes through a four-method seam over `*mgr.Service` (`Query`, `Control`, diff --git a/internal/provision/config.go b/internal/provision/config.go index a96202c..629c082 100644 --- a/internal/provision/config.go +++ b/internal/provision/config.go @@ -155,8 +155,8 @@ func (o Options) restartService(ctx context.Context, service *managedService, pe return nil } -// waitReady dials at once, then every interval, until a dial succeeds or timeout elapses. Network -// errors and 57P03 are retried; any other server error is final. +// waitReady dials at once, then every interval, until a dial succeeds or timeout elapses. Every +// error that is not a server error is retried, and so is 57P03; any other server error is final. func waitReady( ctx context.Context, timeout, interval time.Duration, diff --git a/internal/provision/service_windows.go b/internal/provision/service_windows.go index 0f97c3b..f2d49ae 100644 --- a/internal/provision/service_windows.go +++ b/internal/provision/service_windows.go @@ -59,7 +59,8 @@ type managedService struct { // openService refuses a service the restart could not complete or that is not the server the run // connects to: missing, out of reach, Disabled, not running once a start in progress has -// finished, or registered for another data directory than the one describe reads. +// finished, with no -D in its PathName, or registered for another data directory than the one +// describe reads. func openService( ctx context.Context, name string, From e6e7a4e0dacc7c6f29edb92b896c8f7e14c38fe9 Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:54:50 +0300 Subject: [PATCH 19/20] refactor(provision): cut comments that restate code --- .github/workflows/ci.yml | 17 ++++------------- cmd/semibase/main.go | 5 ++--- internal/provision/config.go | 3 --- internal/provision/config_test.go | 20 +++++++++++--------- internal/provision/service_windows.go | 7 +------ internal/provision/service_windows_test.go | 19 +++++++++---------- 6 files changed, 27 insertions(+), 44 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4032e40..22f3f9c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,17 +28,9 @@ jobs: with: version: v2.12.2 - # The only test of the service sequence itself. The runner image ships - # PostgreSQL 17 as service postgresql-x64-17, Stopped and Disabled, so the - # first run must refuse it with nothing changed. sc.exe start returns - # while the service is still start pending, so the restart run begins - # with openService waiting for Running, the way an installer that has - # just started the service runs site; no other readiness wait precedes - # it. That run ties the service to the server on port 5432 through the - # -D in its PathName, finds settings pending and restarts the service, - # sending the Start only once the old server's address refuses; the next - # finds nothing pending and must not restart it; a service that does not - # exist is refused before any change. + # sc.exe start returns while the service is Start Pending; run site at once + # with no readiness wait, so openService's own wait is what runs. The + # sequence is in docs/architecture/provisioning.md#the-service-restart. - name: Restart the runner's PostgreSQL service through site env: SEMIBASE_SUPER_PASSWORD: root @@ -156,8 +148,7 @@ jobs: ./semibase site --database semiplot_dev --expected-major 17 # The service container is never restarted, so the loopback-only value - # stays pending; a pending value proves site wrote a delta, and the - # postgresql.auto.conf line proves it wrote 'localhost'. + # stays pending. - name: Require the listen_addresses delta env: PGHOST: localhost diff --git a/cmd/semibase/main.go b/cmd/semibase/main.go index 9f0b08b..12c0cef 100644 --- a/cmd/semibase/main.go +++ b/cmd/semibase/main.go @@ -155,9 +155,8 @@ func revisionFromSettings(settings []debug.BuildSetting) string { return vcsRevision } -// One flag set serves both commands, which differ only in the tuning phase; its one flag, -// --restart-service, is registered for site alone. Password flags default to empty so the -// usage output can only ever show the variable names, never their values. +// Password flags default to empty so the usage output can only ever show the variable names, +// never their values. func newFlagSet(command string, options *provision.Options) *flag.FlagSet { flags := flag.NewFlagSet(command, flag.ContinueOnError) flags.SetOutput(errorOutput) diff --git a/internal/provision/config.go b/internal/provision/config.go index 629c082..86f8f71 100644 --- a/internal/provision/config.go +++ b/internal/provision/config.go @@ -120,7 +120,6 @@ func (o Options) applySettings(ctx context.Context) ([]string, error) { return pendingRestartSettings(ctx, conn) } -// restartAdvice is the operator's next step for settings that wait for a restart. func restartAdvice(pending []string) string { advice := "Restart the PostgreSQL service or reboot the machine." if slices.Contains(pending, "listen_addresses") { @@ -155,8 +154,6 @@ func (o Options) restartService(ctx context.Context, service *managedService, pe return nil } -// waitReady dials at once, then every interval, until a dial succeeds or timeout elapses. Every -// error that is not a server error is retried, and so is 57P03; any other server error is final. func waitReady( ctx context.Context, timeout, interval time.Duration, diff --git a/internal/provision/config_test.go b/internal/provision/config_test.go index 126470f..be1d674 100644 --- a/internal/provision/config_test.go +++ b/internal/provision/config_test.go @@ -36,17 +36,20 @@ func TestRestartAdvice(t *testing.T) { var errRefused = errors.New("connection refused") -// scriptedDialer returns a dialer that fails with failure failures times, then returns conn, and -// its call count. -func scriptedDialer(failures int, failure error, conn *pgx.Conn) (func(context.Context) (*pgx.Conn, error), *int) { - calls := 0 - return func(context.Context) (*pgx.Conn, error) { - calls++ - if calls <= failures { +func scriptedDialer( + failures int, + failure error, + conn *pgx.Conn, +) (dial func(context.Context) (*pgx.Conn, error), calls *int) { + calls = new(int) + dial = func(context.Context) (*pgx.Conn, error) { + *calls++ + if *calls <= failures { return nil, failure } return conn, nil - }, &calls + } + return dial, calls } func TestWaitReady(t *testing.T) { @@ -99,7 +102,6 @@ func TestWaitReady(t *testing.T) { } } -// a dial in flight at the deadline fails with the context's error; the message keeps the refusal func TestWaitReadyKeepsTheLastRealErrorAtTheDeadline(t *testing.T) { calls := 0 dial := func(ctx context.Context) (*pgx.Conn, error) { diff --git a/internal/provision/service_windows.go b/internal/provision/service_windows.go index f2d49ae..02bd440 100644 --- a/internal/provision/service_windows.go +++ b/internal/provision/service_windows.go @@ -48,7 +48,6 @@ const listenerDialTimeout = 15 * time.Second type managedService struct { name string control serviceControl - // address is the IP:port the run's connection reached, which listening dials address string // listening reports whether a TCP connect to address succeeds; false with a nil error means // the connect was refused, so no postmaster listens there @@ -58,9 +57,7 @@ type managedService struct { } // openService refuses a service the restart could not complete or that is not the server the run -// connects to: missing, out of reach, Disabled, not running once a start in progress has -// finished, with no -D in its PathName, or registered for another data directory than the one -// describe reads. +// connects to. func openService( ctx context.Context, name string, @@ -336,8 +333,6 @@ func (s *managedService) stoppedAfterStart(ctx context.Context) error { s.name, s.address, s.name) } -// pollUntil queries the service until done accepts its state or ctx ends, and returns the last -// state it saw. func (s *managedService) pollUntil(ctx context.Context, done func(svc.State) bool) (svc.State, error) { var last svc.State for { diff --git a/internal/provision/service_windows_test.go b/internal/provision/service_windows_test.go index a04623b..439a60b 100644 --- a/internal/provision/service_windows_test.go +++ b/internal/provision/service_windows_test.go @@ -28,16 +28,15 @@ var ( // fakeService answers Query with states and the listener probe with probes, each in order and // repeating its last entry; no probes means a refused connect. type fakeService struct { - config mgr.Config - states []svc.State - probes []probeResult - queries int - probeCalls int - controlErr error - startErr error - stops int - starts int - // probe calls made before the Start, and whether the last of them saw the port refused + config mgr.Config + states []svc.State + probes []probeResult + queries int + probeCalls int + controlErr error + startErr error + stops int + starts int probesAtStart int startAfterRefusal bool } From 3de4771522b981771f1143eb08dd67e7a49a2c8f Mon Sep 17 00:00:00 2001 From: Oleg <18466855+EvaTheSalmon@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:07:57 +0300 Subject: [PATCH 20/20] docs(plans): record the site restart run --- .../20261019-site-loopback-and-restart.md | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/docs/plans/20261019-site-loopback-and-restart.md b/docs/plans/20261019-site-loopback-and-restart.md index f4fb990..7c0228e 100644 --- a/docs/plans/20261019-site-loopback-and-restart.md +++ b/docs/plans/20261019-site-loopback-and-restart.md @@ -252,3 +252,32 @@ - On an installation machine, confirm the Simple-Scada project connects to `localhost` or `127.0.0.1`. A project that names the machine's own IP or host name stops writing after the restart. + +**Executed by exec:** +- branch: site-loopback-and-restart + +## Verify it yourself + +1. The loopback delta is written. In CI run 39264258290 (HEAD e6e7a4e), the Linux step "Require + the listen_addresses delta" is green: `pending_restart` for `listen_addresses` is `t` and + `postgresql.auto.conf` holds `localhost`. On `password-hygiene` (3cf9cd9) the setting is not in + `config.go`, so the same queries print `f` and nothing. +2. The restart runs once and only when needed. In the same run, the Windows step "Restart the + runner's PostgreSQL service through site" prints, in order: the Disabled refusal with no + `ALTER SYSTEM SET` line; `state after sc.exe start: StartPending`; the identity line + `service postgresql-x64-17 runs data directory C:\PostgreSQL\17\data, as the server on + [::1]:5432 does`; `restarting service postgresql-x64-17: listen_addresses, shared_buffers + waiting for a restart`; `stopped, nothing answers on [::1]:5432`; `running`; `all settings + active`. The second run prints no restart line, and `no-such-service` is refused. +3. Start always follows an accepted Stop when only the command is interrupted. Run + `go test ./internal/provision -run TestRestart -v` on Windows: the rows "an interrupt after the + Stop still starts the service" and "the command timeout after the Stop still starts the + service" pass, and every row asserts that Start comes only after the port refused a connect. +4. The flag is scoped: `go test ./cmd/semibase -run TestParseOptions -v` shows `bench + --restart-service` refused as an undefined flag, non-loopback and socket hosts refused, and + `localhost`, `127.0.0.1`, `::1` accepted. +5. Manual, on a machine with an EDB PostgreSQL 17 and an elevated PowerShell: run `semibase site + --restart-service postgresql-x64-17` with the passwords in `$env:SEMIBASE_*`, then + `psql -h 127.0.0.1 -U postgres -c "SHOW listen_addresses"` prints `localhost`, and + `Test-NetConnection -Port 5432` fails. The residual cases that can still leave + the service stopped are listed in `docs/architecture/provisioning.md#the-service-restart`.