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

Remove callmemaybe bits from compute #172

Merged
merged 1 commit into from
Jun 10, 2022

Conversation

SomeoneToIgnore
Copy link

@SomeoneToIgnore SomeoneToIgnore commented Jun 2, 2022

Follow-up of neondatabase/neon#1619

@@ -71,25 +71,6 @@ zenith_connect()
errdetail_internal("%s", msg)));
}

/* Ask the Page Server to connect to us, and stream WAL from us. */
if (callmemaybe_connstring && callmemaybe_connstring[0]
Copy link
Member

Choose a reason for hiding this comment

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

BTW it's possible to repurpose this code to help with old ps-1 version that requires callmemaybe.

Compute can send callmemaybe to the pageserver with the following content:

callmemaybe tenant timeline any_sk_address

It will trick old pageserver into connecting to new safekeeper and can work until we implement a more complex migration.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, that's what I've thought was possible and tried to advertise that a few days ago, but so far the consensus was to move on and purge/bump ps-1 after we notify external users about this event.
This way, we don't have to add anything at all in the code 🙂

Choose a reason for hiding this comment

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

@petuhovskiy AFAIK In current setup compute does not send callmemaybe connstr to pageserver. It is the leftover of old setup that allowed to run without safekeepers, so should be ok to remove that entirely.

Copy link
Member

Choose a reason for hiding this comment

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

It is the leftover of old setup that allowed to run without safekeepers, so should be ok to remove that entirely.

Yeah, I'm talking about hacking it to work in a new way.

but so far the consensus was to move on and purge/bump ps-1 after we notify external users about this event.

ps-1 on staging can be deleted, but on production we have to do migration somehow. Hacking callmemaybe in compute can be an alternative.

Copy link
Contributor

Choose a reason for hiding this comment

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

That would be an option, but direct compute->pageserver also has neondatabase/neon#1068 pit (apart from risk of losing data without safekeepers). So yes, let's do migration.

Copy link
Member

Choose a reason for hiding this comment

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

We can don't do direct compute->pageserver START REPLICATION, and just ping pageserver with safekeeper address, so old pageserver will know one safekeeper where it needs to connect.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, indeed

@@ -71,25 +71,6 @@ zenith_connect()
errdetail_internal("%s", msg)));
}

/* Ask the Page Server to connect to us, and stream WAL from us. */
if (callmemaybe_connstring && callmemaybe_connstring[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

That would be an option, but direct compute->pageserver also has neondatabase/neon#1068 pit (apart from risk of losing data without safekeepers). So yes, let's do migration.

@SomeoneToIgnore
Copy link
Author

Looks like the migration is done differently, so merging this.

@SomeoneToIgnore SomeoneToIgnore merged commit 7faa67c into main Jun 10, 2022
@SomeoneToIgnore SomeoneToIgnore deleted the kb/remove-last-callmemaybe branch June 10, 2022 21:44
MMeent pushed a commit that referenced this pull request Jul 7, 2022
MMeent pushed a commit that referenced this pull request Aug 18, 2022
MMeent pushed a commit that referenced this pull request Feb 10, 2023
MMeent pushed a commit that referenced this pull request Feb 10, 2023
MMeent pushed a commit that referenced this pull request May 11, 2023
tristan957 pushed a commit that referenced this pull request Aug 10, 2023
tristan957 pushed a commit that referenced this pull request Nov 8, 2023
tristan957 pushed a commit that referenced this pull request Nov 8, 2023
tristan957 pushed a commit that referenced this pull request Feb 5, 2024
tristan957 pushed a commit that referenced this pull request Feb 5, 2024
tristan957 pushed a commit that referenced this pull request Feb 6, 2024
tristan957 pushed a commit that referenced this pull request May 10, 2024
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.

4 participants