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

Do not perform fast exit for catalog pages in redo filter #6730

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

knizhnik
Copy link
Contributor

Problem

See #6674

Current implementation of neon_redo_read_buffer_filter performs fast exist for catalog pages:

       /*
        * Out of an abundance of caution, we always run redo on shared catalogs,
        * regardless of whether the block is stored in shared buffers. See also
        * this function's top comment.
        */
       if (!OidIsValid(NInfoGetDbOid(rinfo)))
               return false;
*/

as a result last written lsn and relation size for FSM fork are not correctly updated for catalog relations.

## Summary of changes

Do not perform fast path return for catalog relations.

## Checklist before requesting a review

- [ ] I have performed a self-review of my code.
- [ ] If it is a core feature, I have added thorough tests.
- [ ] Do we need to implement analytics? if so did you add the relevant metrics to the dashboard?
- [ ] If this PR requires public announcement, mark it with /release-notes label and add several sentences in this section.

## Checklist before merging

- [ ] Do not forget to reformat commit message to not include the above checklist

@knizhnik knizhnik requested a review from a team as a code owner February 12, 2024 15:46
@knizhnik knizhnik requested review from save-buffer and removed request for a team February 12, 2024 15:46
@knizhnik knizhnik changed the title Do not perform fast exist for catalog pages in redo filter Do not perform fast exit for catalog pages in redo filter Feb 12, 2024
Copy link

github-actions bot commented Feb 12, 2024

2430 tests run: 2318 passed, 0 failed, 112 skipped (full report)


Flaky tests (2)

Postgres 15

  • test_pg_regress[4]: debug
  • test_empty_tenant_size: debug

Code coverage (full report)

  • functions: 54.9% (11884 of 21653 functions)
  • lines: 82.1% (66243 of 80716 lines)

The comment gets automatically updated with the latest test results
c639aa5 at 2024-02-12T20:34:43.386Z :recycle:

@knizhnik knizhnik merged commit b6e070b into main Feb 13, 2024
50 checks passed
@knizhnik knizhnik deleted the redo_catalog_filter branch February 13, 2024 18:41
@hlinnaka
Copy link
Contributor

I don't understand what the problem was. Can you walk me through the problem? Can you write a test case, please?

@knizhnik
Copy link
Contributor Author

Sorry, the problem is explained in description of this PR.
It can be reproduced using this test:

import time
from fixtures.log_helper import log
from fixtures.neon_fixtures import Endpoint, NeonEnv, wait_replica_caughtup


def test_cresatedb_repl(neon_simple_env: NeonEnv):
    env = neon_simple_env

    with env.endpoints.create_start(
        branch_name="main",
        endpoint_id="primary",
    ) as primary:
        time.sleep(1)
        with env.endpoints.new_replica_start(origin=primary, endpoint_id="secondary") as secondary:
            with primary.cursor() as p_cur:
                p_cur.execute("CREATE DATABASE empty")
            with primary.cursor(dbname="empty") as p_cur:
                 p_cur.execute("alter database empty set timezone to 'Australia/Sydney'")
            wait_replica_caughtup(primary, secondary)
            with secondary.cursor(dbname="empty") as s_cur:
                s_cur.execute("select count(*) from pg_class")

Can I add it to PR #6705 (which fix function wait_replica_caughtup and moves it to fixtures)?

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