Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unlogged index fix v14 #259

Merged
merged 3 commits into from
Feb 22, 2023
Merged

Unlogged index fix v14 #259

merged 3 commits into from
Feb 22, 2023

Conversation

knizhnik
Copy link

@knizhnik knizhnik changed the base branch from REL_15_STABLE_neon to REL_14_STABLE_neon February 20, 2023 16:50
@arssher
Copy link

arssher commented Feb 21, 2023

Checked with

pkill pageserver; pkill postgres; pkill safekeeper; pkill pytest; rm -rf .neon/ && ./target/debug/neon_local init  && target/debug/neon_local start && ./target/debug/neon_local tenant create --set-default --pg-version 14 && ./target/debug/neon_local pg start main --pg-version 14
psql -h localhost -p 55432 -U cloud_admin postgres
psql (15.1, server 14.6)
Type "help" for help.

localhost:55432 cloud_admin@postgres:1711027=# create unlogged table iut (title int); CREATE UNIQUE INDEX title_idx ON iut (title); insert into  iut values (12);

create unlogged table iut (title int); CREATE UNIQUE INDEX title_idx ON iut (title); insert into  iut values (12);
./target/debug/neon_local pg stop main; ./target/debug/neon_local pg start main

Without patch table iut; fails, with patch it is fine.

Relation heap;
DropRelFileNodesAllBuffers(&index->rd_smgr, 1);
heap = RelationIdGetRelation(index->rd_index->indrelid);
index->rd_indam->ambuild(heap, index, BuildIndexInfo(index));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not confident we're OK to build an index without locking the base relation at a similar level as normal index creation.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And perhaps index relation should be locked in AccessExclusiveLock like index_create does.
Though the risks here are incredibly small.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, please check once again

@knizhnik knizhnik requested a review from MMeent February 21, 2023 16:55
@knizhnik knizhnik merged commit 125d0bd into REL_14_STABLE_neon Feb 22, 2023
@knizhnik knizhnik deleted the unlogged_index_fix_v14 branch February 22, 2023 15:37
arssher pushed a commit that referenced this pull request Feb 22, 2023
* Avoid errors when accessing indexes of unlogge tables after compute restart

* Address review complaints: add comment to mdopenfork

* Initialize unlogged index undex eclusive lock

This is a hotfix cherry picking 125d0bd to release the fix earlier.
arssher added a commit to neondatabase/neon that referenced this pull request Feb 23, 2023
Release 2023-02-23

Hotfix for the unlogged tables with indexes issue.

neondatabase/postgres#259
neondatabase/postgres#262
MMeent pushed a commit that referenced this pull request Feb 23, 2023
* Avoid errors when accessing indexes of unlogge tables after compute restart

* Address review complaints: add comment to mdopenfork

* Initialize unlogged index undex eclusive lock
@arssher arssher mentioned this pull request Feb 24, 2023
arssher pushed a commit to neondatabase/neon that referenced this pull request Feb 24, 2023
Instead of trying to create missing files on the way, send init fork contents as
main fork from pageserver during basebackup. Add test for that. Call
put_rel_drop for init forks; previously they weren't removed. Bump
vendor/postgres to revert previous approach on Postgres side.

Co-authored-by: Arseny Sher <[email protected]>

ref neondatabase/postgres#264
ref neondatabase/postgres#259
ref #1222
arssher pushed a commit to neondatabase/neon that referenced this pull request Feb 24, 2023
Instead of trying to create missing files on the way, send init fork contents as
main fork from pageserver during basebackup. Add test for that. Call
put_rel_drop for init forks; previously they weren't removed. Bump
vendor/postgres to revert previous approach on Postgres side.

Co-authored-by: Arseny Sher <[email protected]>

ref neondatabase/postgres#264
ref neondatabase/postgres#259
ref #1222
arssher pushed a commit to neondatabase/neon that referenced this pull request Feb 24, 2023
Instead of trying to create missing files on the way, send init fork contents as
main fork from pageserver during basebackup. Add test for that. Call
put_rel_drop for init forks; previously they weren't removed. Bump
vendor/postgres to revert previous approach on Postgres side.

Co-authored-by: Arseny Sher <[email protected]>

ref neondatabase/postgres#264
ref neondatabase/postgres#259
ref #1222
MMeent pushed a commit that referenced this pull request May 11, 2023
* Avoid errors when accessing indexes of unlogge tables after compute restart

* Address review complaints: add comment to mdopenfork

* Initialize unlogged index undex eclusive lock
tristan957 pushed a commit that referenced this pull request Aug 10, 2023
* Avoid errors when accessing indexes of unlogge tables after compute restart

* Address review complaints: add comment to mdopenfork

* Initialize unlogged index undex eclusive lock
tristan957 pushed a commit that referenced this pull request Nov 8, 2023
* Avoid errors when accessing indexes of unlogge tables after compute restart

* Address review complaints: add comment to mdopenfork

* Initialize unlogged index undex eclusive lock
tristan957 pushed a commit that referenced this pull request Nov 8, 2023
* Avoid errors when accessing indexes of unlogge tables after compute restart

* Address review complaints: add comment to mdopenfork

* Initialize unlogged index undex eclusive lock
tristan957 pushed a commit that referenced this pull request Feb 5, 2024
* Avoid errors when accessing indexes of unlogge tables after compute restart

* Address review complaints: add comment to mdopenfork

* Initialize unlogged index undex eclusive lock
tristan957 pushed a commit that referenced this pull request Feb 5, 2024
* Avoid errors when accessing indexes of unlogge tables after compute restart

* Address review complaints: add comment to mdopenfork

* Initialize unlogged index undex eclusive lock
tristan957 pushed a commit that referenced this pull request Feb 6, 2024
* Avoid errors when accessing indexes of unlogge tables after compute restart

* Address review complaints: add comment to mdopenfork

* Initialize unlogged index undex eclusive lock
tristan957 pushed a commit that referenced this pull request May 10, 2024
* Avoid errors when accessing indexes of unlogge tables after compute restart

* Address review complaints: add comment to mdopenfork

* Initialize unlogged index undex eclusive lock
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants