Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1sks0M-003Rnl-Kd for pgsql-hackers@arkaria.postgresql.org; Sun, 01 Sep 2024 21:21:03 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1sks0L-002Mcb-C8 for pgsql-hackers@arkaria.postgresql.org; Sun, 01 Sep 2024 21:21:01 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1sks0K-002McT-QK for pgsql-hackers@lists.postgresql.org; Sun, 01 Sep 2024 21:21:01 +0000 Received: from mail-ej1-x62c.google.com ([2a00:1450:4864:20::62c]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1sks0H-000BWh-UB for pgsql-hackers@postgresql.org; Sun, 01 Sep 2024 21:20:59 +0000 Received: by mail-ej1-x62c.google.com with SMTP id a640c23a62f3a-a867a564911so423877966b.2 for ; Sun, 01 Sep 2024 14:20:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1725225656; x=1725830456; darn=postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=0EQ8GvZwrYomWP1nYwVOiLlpGvdkqYQih/eQrYhXhk0=; b=SeIxFvV1bsEZhF+hPWDpiTzTKgVW9le8BYD533J1dqRlXEUcHRc3+CLdWn8cvPcjbr DIidHzHbGVSQfASzbhardrWQxqjyFJt3wf8XnGqyPE9aBFIVS/asGnz5Smwj+rNd2WKL lpa7nrR0E+5LFoxbFKEGfyc1CMNz+pJZ2wnklYDgEL2KgdwI3Go0CvZmf1zWh+f7ZxbV 6QIeG/yg/y24jqpceMAo/m9L+n5GnBff49KFyB4p2vn41vURDh0fBl4byvsRRE+nH1z6 Y0ghLsd1fXjSBz4NeEu8IfQdvr6mMYNUFve53L67S+WQ7PwFsCqxw2xZMSmfwM1Fvg7L dU8A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725225656; x=1725830456; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=0EQ8GvZwrYomWP1nYwVOiLlpGvdkqYQih/eQrYhXhk0=; b=acM8EbEcRP+TrqjIawWhpfD8nySlRcLU9p9yMNKMKriFoddUMAtIQodcf5gNGinocW 9Nu/S/XjoFIWMio31JQbaNN8r8rVKeuaRY70BCpl9s0xZCbXFlj8DUj80S2yLlT4SzU8 sAzrGCn4irpNQ+4y22yg64yS9oQCgy5tV8EFDSq4H65CQTSXBQaNnBFg0dgd7grR274z madAEPMG/DcylEfH8Or3iaf8xK0Efm5Lu+kPDfsh8quRkfdOJqVSzOM89avM64LKLMu/ awUB8Nkth6NRXFgbpTtXfj96p0/xR9pivIpODUnZyFnGmZz+2siNtmUaJXnZmhSkFq1U hVbg== X-Gm-Message-State: AOJu0YwcL8Qpmw8+rKkqHtfErSJWVD1QdHZNJfKKvnvR/a48VG8n5XcP hSORaJn2IWheN94Y+aJSsGOztqKm5oTt1iD9AagBOFSTEOWB9iywDsFyGWg+8MkxnTbb0KW9dMf 0ZC4IE1ibsAi2hSPhs5Kyqekoh+o= X-Google-Smtp-Source: AGHT+IE1QcjERJH49XltWUiSswBtUzvh0CJkcvzqCHH9BarSvOkQWxesVXUXXHVPlK2+DhPYuZh2yPvJiikEIq84T/I= X-Received: by 2002:a17:906:c14b:b0:a72:b4d6:ec6c with SMTP id a640c23a62f3a-a89b955a9cemr374340366b.33.1725225655187; Sun, 01 Sep 2024 14:20:55 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Michail Nikolaev Date: Sun, 1 Sep 2024 23:19:00 +0200 Message-ID: Subject: Re: Revisiting {CREATE INDEX, REINDEX} CONCURRENTLY improvements To: Matthias van de Meent Cc: PostgreSQL Hackers , Andrey Borodin , Melanie Plageman Content-Type: multipart/alternative; boundary="00000000000055be5506211566e3" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --00000000000055be5506211566e3 Content-Type: text/plain; charset="UTF-8" Hello, Matthias! Just wanted to update you with some information about the next steps in work. > In heapam_index_build_range_scan, it seems like you're popping the > snapshot and registering a new one while holding a tuple from > heap_getnext(), thus while holding a page lock. I'm not so sure that's > OK, expecially when catalogs are also involved (specifically for > expression indexes, where functions could potentially be updated or > dropped if we re-create the visibility snapshot) I have returned to the solution with a dedicated catalog_xmin for backends [1]. Additionally, I have added catalog_xmin to pg_stat_activity [2]. > In heapam_index_build_range_scan, you pop the snapshot before the > returned heaptuple is processed and passed to the index-provided > callback. I think that's incorrect, as it'll change the visibility of > the returned tuple before it's passed to the index's callback. I think > the snapshot manipulation is best added at the end of the loop, if we > add it at all in that function. Now it's fixed, and the snapshot is reset between pages [3]. Additionally, I resolved the issue with potential duplicates in unique indexes. It looks a bit clunky, but it works for now [4]. Single commit from [5] also included, just for stable stress testing. Full diff is available at [6]. Best regards, Mikhail. [1]: https://github.com/michail-nikolaev/postgres/commit/01a47623571592c52c7a367f85b1cff9d8b593c0 [2]: https://github.com/michail-nikolaev/postgres/commit/d3345d60bd51fe2e0e4a73806774b828f34ba7b6 [3]: https://github.com/michail-nikolaev/postgres/commit/7d1dd4f971e8d03f38de95f82b730635ffe09aaf [4]: https://github.com/michail-nikolaev/postgres/commit/4ad56e14dd504d5530657069068c2bdf172e482d [5]: https://commitfest.postgresql.org/49/5160/ [6]: https://github.com/postgres/postgres/compare/master...michail-nikolaev:postgres:new_index_concurrently_approach?diff=split&w= --00000000000055be5506211566e3 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hello, Matthias!

Just wanted to update you with som= e information about the next steps in work.

> In heapam_index_bui= ld_range_scan, it seems like you're popping the
> snapshot and re= gistering a new one while holding a tuple from
> heap_getnext(), thus= while holding a page lock. I'm not so sure that's
> OK, expe= cially when catalogs are also involved (specifically for
> expression= indexes, where functions could potentially be updated or
> dropped i= f we re-create the visibility snapshot)

I have returned to the solut= ion with a dedicated catalog_xmin for backends [1].
Additionally, I have= added catalog_xmin to pg_stat_activity [2].

> In heapam_index_bu= ild_range_scan, you pop the snapshot before the
> returned heaptuple = is processed and passed to the index-provided
> callback. I think tha= t's incorrect, as it'll change the visibility of
> the return= ed tuple before it's passed to the index's callback. I think
>= ; the snapshot manipulation is best added at the end of the loop, if we
= > add it at all in that function.

Now it's fixed, and the sna= pshot is reset between pages [3].

Additionally, I resolved the issue= with potential duplicates in unique indexes. It looks a bit clunky, but it= works for now [4].

Single commit from [5] also included= , just for stable stress testing.

Full diff is ava= ilable at [6].

Best regards,
Mikhail.

[3]: https://github.com/michail-nikolaev/postgres/co= mmit/7d1dd4f971e8d03f38de95f82b730635ffe09aaf
--00000000000055be5506211566e3--