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

Fix test to be more robust with slow pageserver. #2131

Merged
merged 1 commit into from
Jul 20, 2022

Conversation

hlinnaka
Copy link
Contributor

If the WAL arrives at the pageserver slowly, it's possible that the
branch is created before all the data on the parent branch have
arrived. That results in a failure:

test_runner/batch_others/test_tenant_relocation.py:259: in test_tenant_relocation
    timeline_id_second, current_lsn_second = populate_branch(pg_second, create_table=False, expected_sum=1001000)
test_runner/batch_others/test_tenant_relocation.py:133: in populate_branch
    assert cur.fetchone() == (expected_sum, )
E   assert (500500,) == (1001000,)
E     At index 0 diff: 500500 != 1001000
E     Full diff:
E     - (1001000,)
E     + (500500,)

To fix, specify the LSN to branch at, so that the pageserver will wait
for it arrive.

See #2063

If the WAL arrives at the pageserver slowly, it's possible that the
branch is created before all the data on the parent branch have
arrived. That results in a failure:

    test_runner/batch_others/test_tenant_relocation.py:259: in test_tenant_relocation
        timeline_id_second, current_lsn_second = populate_branch(pg_second, create_table=False, expected_sum=1001000)
    test_runner/batch_others/test_tenant_relocation.py:133: in populate_branch
        assert cur.fetchone() == (expected_sum, )
    E   assert (500500,) == (1001000,)
    E     At index 0 diff: 500500 != 1001000
    E     Full diff:
    E     - (1001000,)
    E     + (500500,)

To fix, specify the LSN to branch at, so that the pageserver will wait
for it arrive.

See #2063
@hlinnaka
Copy link
Contributor Author

This fixes the flaky test discussed at #2063. I didn't comb through all the tests creating branches, though, so there might well be more instances of this. A more complete fix would be to always require an LSN when creating a branch, as discussed at #2063. So I don't consider this as a full fix for that issue.

Copy link
Member

@ololobus ololobus left a comment

Choose a reason for hiding this comment

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

Didn't know that create branch already accepts LSN. This should works as far as I see, but that could be I don't know some details

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.

2 participants