Added support for concurrent index creation for Postgres versions above 13

[danilfilatoff]

According to Postgres documentation it's not allowed to create indexes concurrently inside a transaction block which may be any other expression. Basically now we faced with an issue when we don't have any chance to create an index concurrently. This PR attempts to solve this. Beside unit tests I tested out this solution with our current delta.

Regular index builds permit other regular index builds on the same table to occur simultaneously, but only one concurrent index build can occur on a table at a time. In either case, schema modification of the table is not allowed while the index is being built. Another difference is that a regular CREATE INDEX command can be performed within a transaction block, but CREATE INDEX CONCURRENTLY cannot.

[danilfilatoff]

@oskardudycz Could you please take a look?

[mysticmind]

@danilfilatoff I have enabled to run the CI now. Quick question, with regards to the fix which you done, by adding a comment in between the index statements, it does not throw an error?

[mysticmind]

@danilfilatoff a unit test write_unique_and_concurrent_index_with_distinct_nulls is failing, please take a look.

[danilfilatoff]

@mysticmind thanks for the response. Regarding comments, I don't see a reason why adding comments may throw an error. The comments are added between statements and statements are executed separately, so no, I haven't faced with this issue. Regarding the unit test, before the last changes in master it was easy to run all tests without errors, now in the master I can't manage to pass all unit tests. Probably I just need to configure a run somehow. There is always about 20 tests failing. That's why I missed that case(write_unique_and_concurrent_index_with_distinct_nulls). Anyway it's fixed now.

[mysticmind]

thanks for the response. Regarding comments, I don't see a reason why adding comments may throw an error. The comments are added between statements and statements are executed separately, so no, I haven't faced with this issue.

@danilfilatoff My comment was rather a question/clarification in terms of fix what you did to wrap a comment block. I understand now that the comment block allows to segregate the concurrent command to run seperately.

[danilfilatoff]

@oskardudycz added. I had to rewrite a method inside IntegrationContext as it couldn't execute a whole migration in one statement with concurrency.

[danilfilatoff]

I'll check failed tests.

[danilfilatoff]

Can someone run CI again please? If my last commit can't fix the failed tests then I'll need a hand.

[mysticmind]

Can someone run CI again please? If my last commit can't fix the failed tests then I'll need a hand.

Initiated CI run.

[danilfilatoff]

Fanny thing I can't reproduce it locally.

[mysticmind]

@danilfilatoff I will take a look and keep you posted.

[mysticmind]

@danilfilatoff I have ran few tests and confirming that the failed tests are not due your PR changes. I am trying to fix the failing tests, will keep you posted in a little while.

[oskardudycz]

@mysticmind, @danilfilatoff From what I see; tests are green on the master branch. They're failing on the branch here, even with retries.

[oskardudycz]

@danilfilatoff after I brought back the original integration context state, flaky tests started to work fine; I also turned on a few more tests. Yet, either I'm missing something, or the provided fix seems not to be working; see failing test:

[xUnit.net 00:00:02.33]     Weasel.Postgresql.Tests.Tables.detecting_table_deltas.detect_different_index_with_concurrent_creation [FAIL]
[xUnit.net 00:00:02.33]       System.Exception : DDL Execution Failure.
[xUnit.net 00:00:02.33]       DROP TABLE IF EXISTS deltas.people CASCADE;
[xUnit.net 00:00:02.33]       CREATE TABLE deltas.people (
[xUnit.net 00:00:02.33]           id                         integer                        NOT NULL,
[xUnit.net 00:00:02.33]           first_name                 varchar                        NULL,
[xUnit.net 00:00:02.33]           last_name                  varchar                        NULL,
[xUnit.net 00:00:02.33]           user_name                  varchar                        NULL,
[xUnit.net 00:00:02.33]           created_datetime           timestamp without time zone    NULL,
[xUnit.net 00:00:02.33]           created_datetime_offset    timestamp with time zone       NULL,
[xUnit.net 00:00:02.33]           data                       jsonb                          NULL,
[xUnit.net 00:00:02.33]       CONSTRAINT pkey_people_id PRIMARY KEY (id)
[xUnit.net 00:00:02.33]       );
[xUnit.net 00:00:02.33]       
[xUnit.net 00:00:02.33]       --WEASEL_INDEX_CREATION_BEGIN
[xUnit.net 00:00:02.33]       CREATE UNIQUE INDEX CONCURRENTLY idx_people_user_name ON deltas.people USING btree (user_name);
[xUnit.net 00:00:02.33]       --WEASEL_INDEX_CREATION_END
[xUnit.net 00:00:02.33]       
[xUnit.net 00:00:02.33]       ---- Npgsql.PostgresException : 25001: CREATE INDEX CONCURRENTLY cannot be executed within a pipeline
[xUnit.net 00:00:02.33]       Stack Trace:
[xUnit.net 00:00:02.33]         /home/runner/work/weasel/weasel/src/Weasel.Postgresql.Tests/IntegrationContext.cs(50,0): at Weasel.Postgresql.Tests.IntegrationContext.CreateSchemaObjectInDatabase(ISchemaObject schemaObject)
[xUnit.net 00:00:02.33]         /home/runner/work/weasel/weasel/src/Weasel.Postgresql.Tests/Tables/detecting_table_deltas.cs(227,0): at Weasel.Postgresql.Tests.Tables.detecting_table_deltas.detect_different_index(Boolean withDistinctNulls, Boolean isConcurrent)
[xUnit.net 00:00:02.33]         --- End of stack trace from previous location ---
[xUnit.net 00:00:02.33]         ----- Inner Stack Trace -----
[xUnit.net 00:00:02.33]            at Npgsql.Internal.NpgsqlConnector.<ReadMessage>g__ReadMessageLong|233_0(NpgsqlConnector connector, Boolean async, DataRowLoadingMode dataRowLoadingMode, Boolean readingNotifications, Boolean isReadingPrependedMessage)
[xUnit.net 00:00:02.33]            at Npgsql.NpgsqlDataReader.NextResult(Boolean async, Boolean isConsuming, CancellationToken cancellationToken)
[xUnit.net 00:00:02.33]            at Npgsql.NpgsqlDataReader.NextResult(Boolean async, Boolean isConsuming, CancellationToken cancellationToken)
[xUnit.net 00:00:02.33]            at Npgsql.NpgsqlCommand.ExecuteReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken)
[xUnit.net 00:00:02.33]            at Npgsql.NpgsqlCommand.ExecuteReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken)
[xUnit.net 00:00:02.33]            at Npgsql.NpgsqlCommand.ExecuteNonQuery(Boolean async, CancellationToken cancellationToken)
[xUnit.net 00:00:02.33]         /home/runner/work/weasel/weasel/src/Weasel.Postgresql.Tests/IntegrationContext.cs(45,0): at 

I understand that you can only run concurrent index creation as a separate statement. And you cannot run it inside the transaction, which is created implicitly by npgsql while running the command.

@danilfilatoff, thoughts?

[danilfilatoff]

The problem in tests I was trying to fix by changing Integration context is that the schema creation process is executed differently than in production code. I think it's good idea also in general to unify a bit tests so they are executed as in production. And it matters here for my other changes. It's very expected that the fix stopped working. Another problem that I can't make all tests pass on my local computers even on master, so it really hinders my possibilities to change code. That's why I've been asking for help with it.

[oskardudycz]

@danilfilatoff, are you using Postgres 15.3? How are you starting your postgres? If just running docker-compose, then by default, is running a 12.2 version, you need to uncomment this line and comment out the older version: https://github.com/JasperFx/weasel/blob/master/docker-compose.yml#L5

You need to do that, as the changes to concurrent indexes were introduced in PG 13 afair.

Regarding

The problem in tests I was trying to fix by changing Integration context is that the schema creation process is executed differently than in production code

Are you referring to your production code or Marten?

[danilfilatoff]

Yep, I use docker compose, and I tried different versions of Postgres, it didn't help much. Regarding applying changes I'm referring to Weasel method ApplyAllConfiguredChangesToDatabaseAsync which is also used in marten when ShouldApplyChangesOnStartup=true https://github.com/JasperFx/marten/blob/21e172af3915123b1274926de98b516223b506bd/src/Marten/Services/MartenActivator.cs#L80C30-L80C30

[danilfilatoff]

Please run tests again, it seems like I managed to fix all current issues with tests.

[danilfilatoff]

Only one test is failed, I think it's much better already. And this test case passes locally even using a new Macbook where 92 tests can never pass. I think it should work with a retry. Also could you please run tests here using postgres15.3?

[mysticmind]

@danilfilatoff I will trigger the CI again.

[mysticmind]

@danilfilatoff everything is green now 👍

[mysticmind]

@oskardudycz Please check and merge the PR.