Skip to content

test: make PostgreSQL RESET ALL locked-hostgroup TAP test independent of pooled startup state #6160

Description

@renecannao

Summary

pgsql-set_statement_test-t is flaky in the "RESET ALL with locked hostgroup in pipeline mode" case. The test assumes that a newly opened frontend connection also receives a newly created PostgreSQL backend connection. That is not guaranteed under multiplexing.

A physical backend can have been created earlier with a non-default startup option such as DateStyle=SQL,DMY. A later default frontend client correctly receives a live DateStyle=ISO, MDY session, but RESET ALL must still be rejected while the hostgroup is locked: PostgreSQL would reset the physical connection to its original SQL, DMY startup value, not to the later client's default.

The current rejection is therefore safety-preserving. The TAP test is flaky because its “startup values match” precondition is not established.

This was the sole TAP failure in PR #6151's CI run: run 33402499119.

Failure

The failing TAP assertion is:

not ok 70 - RESET ALL with locked hostgroup in pipeline mode

Client error:

ERROR: RESET ALL is not supported when hostgroup is locked and connection startup values differ. Use SET to explicitly set the desired values.

ProxySQL log:

RESET ALL is not allowed when hostgroup is locked and startup parameter values differ between client and backend. Mismatched variables: DateStyle

Why this is not caused by PR #6151

The relevant PostgreSQL paths are unchanged between the PR base and the failing head:

  • lib/PgSQL_Session.cpp
  • lib/PgSQL_Connection.cpp
  • lib/PgSQL_HostGroups_Manager.cpp
  • test/tap/tests/pgsql-set_statement_test-t.cpp

The source hash of pgsql-set_statement_test-t.cpp was also identical in the base, the failing PR head, and a known passing head. A full replay of the legacy-g4 group with CI-style noise and no cluster nodes passed, confirming that pool history/timing controls whether the test sees the stale physical backend.

Root cause

1. Backend startup values are captured when the physical connection is created

When ProxySQL creates a PostgreSQL backend connection, it copies the critical client variables into the physical backend connection's startup-parameter state:

myds->sess->mybe->server_myds->myconn->copy_pgsql_variables_to_startup_parameters(true);

This occurs in lib/PgSQL_Connection.cpp. If that client connected with options='-c DateStyle=SQL,DMY', the physical backend records SQL, DMY as its startup value.

2. A later default client can reuse that physical backend

On reuse, ProxySQL synchronizes the live backend session to the later client. The later default client consequently sees:

SHOW DateStyle;
-- ISO, MDY

That live value is correct, but it does not change the physical PostgreSQL connection's original startup value.

3. RESET ALL is correctly blocked in the locked-hostgroup pipeline path

In lib/PgSQL_Session.cpp, the extended-query, locked-hostgroup RESET path compares the client and physical-backend startup hashes. It rejects RESET ALL if any critical startup parameter differs.

This is necessary: forwarding RESET ALL would make PostgreSQL reset DateStyle to the physical backend's SQL, DMY, thereby violating the default client's state. The error asks the client to use explicit SET statements instead.

4. The test's assumption is invalid

test_reset_all_locked_hostgroup_pipeline() currently says that a “fresh connection” has matching startup values. It creates a fresh frontend connection only. Its selected physical backend can still be pooled and can have been created by an earlier client with different startup options.

Deterministic reproduction

The following sequence was performed against a dedicated, isolated PostgreSQL TAP stack. It changes runtime values only and restores them in finally; it does not execute any SAVE ... TO DISK or LOAD ... FROM DISK command.

Controlled setup

  1. Snapshot every pgsql_servers.max_connections value and pgsql-free_connections_pct.
  2. Set all PostgreSQL hostgroups to max_connections=1 and load the change to runtime.
  3. Set pgsql-free_connections_pct=0, load PostgreSQL variables to runtime, and wait until SUM(ConnFree)=0.
  4. Set pgsql-free_connections_pct=100 and load PostgreSQL variables to runtime. This keeps the one newly-created physical backend available for the next client.

Reproduction

  1. Connect through ProxySQL using:

    options='-c DateStyle=SQL,DMY'
    

    Execute SHOW DateStyle to force backend acquisition, then close the connection.

    Observed:

    first-client DateStyle: SQL, DMY
    stats_pgsql_connection_pool: hostgroup 0, ConnUsed=0, ConnFree=1
    
  2. Connect as the normal default client. Execute:

    SHOW DateStyle;

    Observed:

    ISO, MDY
    

    This proves ProxySQL correctly synchronized the live backend session for the default client.

  3. On that same default connection, enter PostgreSQL pipeline mode and submit:

    SET myapp.lock_var = 'lock_value';
    RESET ALL;
  4. Synchronize and consume pipeline results.

    Observed:

    FeatureNotSupported:
    RESET ALL is not supported when hostgroup is locked and connection startup values differ.
    Use SET to explicitly set the desired values.
    

Repeatability

With the forced single-backend setup and retained free backend, the reproduction produced the same rejection in 3/3 consecutive attempts:

attempt 1: first=SQL, DMY; second=ISO, MDY; RESET ALL=FeatureNotSupported
attempt 2: first=SQL, DMY; second=ISO, MDY; RESET ALL=FeatureNotSupported
attempt 3: first=SQL, DMY; second=ISO, MDY; RESET ALL=FeatureNotSupported

The important controls are:

  • The first client must execute a query; merely completing the frontend handshake does not acquire a backend.
  • The temporary backend must be retained after the first client closes; otherwise the reaper can remove it and the second client may create a fresh default backend.
  • The second client must execute SHOW DateStyle before pipeline mode to prove the live session is default even though the startup-hash comparison will reject RESET ALL.

Proposed fix

Make test_reset_all_locked_hostgroup_pipeline() establish its documented precondition instead of relying on ambient pool state:

  1. Snapshot max_connections per hostgroup and pgsql-free_connections_pct.
  2. Use runtime-only admin changes to drain free PostgreSQL connections before opening the test's default client.
  3. Create the test client only after the pool is empty, so its physical backend is created with the same default startup values as the client.
  4. Run the existing locked-hostgroup pipeline sequence and assert that RESET ALL succeeds.
  5. Restore every snapshotted runtime value on every exit path.
  6. Add a regression arrangement that deliberately creates a non-default-startup physical backend before the cleanup step. That makes the prior test setup fail with the exact CI error and demonstrates why the cleanup is required.

This keeps the server's protective behavior unchanged and turns the test from pool-history-dependent into deterministic.

Acceptance criteria

  • The TAP test reproduces the stale-startup backend condition before its cleanup/precondition step.
  • The test explicitly drains the PostgreSQL free pool before asserting the matching-startup success case.
  • The test validates that the default client sees ISO, MDY before its pipeline commands.
  • The locked-hostgroup pipeline SET myapp.lock_var + RESET ALL success assertion is deterministic.
  • Runtime configuration is restored per hostgroup; no blanket restore may overwrite distinct hostgroup values.
  • TAP sources contain no SAVE ... TO DISK or LOAD ... FROM DISK commands.
  • The focused test is run through test/infra/control/run-tests-isolated.bash after the documented debug and TAP builds.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions