Re: Online enabling of checksums

Hi,

On 2018-04-06 02:28:17 +0200, Daniel Gustafsson wrote:
> Applying this makes the _cancel test pass, moving the failure instead to the
> following _enable test (which matches what coypu and mylodon are seeing).

FWIW, I'm somewhat annoyed that I'm now spending time debugging this to
get the buildfarm green again.

I'm fairly certain that the bug here is a simple race condition in the
test (not the main code!):

The flag informing whether the worker has started is cleared via an
on_shmem_exit() hook:

static void
launcher_exit(int code, Datum arg)
{
    ChecksumHelperShmem->abort = false;
    pg_atomic_clear_flag(&ChecksumHelperShmem->launcher_started);
}

but the the wait in the test is done via functions like:

    CREATE OR REPLACE FUNCTION test_checksums_on() RETURNS boolean AS $$
    DECLARE
        enabled boolean;
    BEGIN
        LOOP
            SELECT setting = 'on' INTO enabled FROM pg_catalog.pg_settings WHERE name = 'data_checksums';
            IF enabled THEN
                EXIT;
            END IF;
            PERFORM pg_sleep(1);
        END LOOP;
        RETURN enabled;
    END;
    $$ LANGUAGE plpgsql;

    INSERT INTO t1 (b, c) VALUES (generate_series(1,10000), 'starting values');

    CREATE OR REPLACE FUNCTION test_checksums_off() RETURNS boolean AS $$
    DECLARE
        enabled boolean;
    BEGIN
        PERFORM pg_sleep(1);
        SELECT setting = 'off' INTO enabled FROM pg_catalog.pg_settings WHERE name = 'data_checksums';
        RETURN enabled;
    END;
    $$ LANGUAGE plpgsql;

which just waits for setting checksums to have finished.  It's
exceedingly unsurprising that a 'pg_sleep(1)' is not a reliable way to
make sure that a process has finished exiting.  Then followup tests fail
because the process is still running

Also:
    CREATE OR REPLACE FUNCTION reader_loop() RETURNS boolean AS $$
    DECLARE
        counter integer;
    BEGIN
        FOR counter IN 1..30 LOOP
            PERFORM count(a) FROM t1;
            PERFORM pg_sleep(0.2);
        END LOOP;
        RETURN True;
    END;
    $$ LANGUAGE plpgsql;
}

really?  Let's just force the test take at least 6s purely from
sleeping?

Greetings,

Andres Freund