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

[#224] Prefill a new server when doing a switch-to #225

Closed
wants to merge 1 commit into from

Conversation

fluca1978
Copy link
Collaborator

When a switch-to operation is issued, the old server is flushed
(i.e., connections are closed) and new connections to the new primary
server are established.
When such connections are established, the system does a full prefill
against the new primary, so that when the switch-to completes, the
new primary is prefilled as the old one was.

Adds also a better detail about the server that is going to be
flushed.

Close #224

When a `switch-to` operation is issued, the old server is flushed
(i.e., connections are closed) and new connections to the new primary
server are established.
When such connections are established, the system does a full prefill
against the new primary, so that when the `switch-to` completes, the
new primary is prefilled as the old one was.

Adds also a better detail about the server that is going to be
flushed.

Close agroal#224
@jesperpedersen
Copy link
Collaborator

Looking at it - there are 2 places where pgagroal_flush_server is called from - 1) The failover scenario, and 2) An unresponsive primary.

For 1) we know that a new primary has been selected. For 2) we know that the primary is the same.

So, I think we can just change pgagroal_flush_server to check for the primary (pgagroal_get_primary) and if it is different than the value passed in invoke pgagroal_prefill(true). That way we don't have to change the function signatures, and it should be a 3-4 line patch :)

Thoughts ?

@jesperpedersen jesperpedersen added the enhancement Improvement to an existing feature label May 4, 2022
fluca1978 added a commit to fluca1978/pgagroal that referenced this pull request May 4, 2022
Whenever the primary host is changed, by means of an explicit
`switch-to` command or by a primary failure, the connection flushing
is activated. If possible, the prefill should be also restored on the
new server.
As suggested in
<agroal#225 (comment)>
it would be nice to check if the specified new server is the same as
the old one (failure of the primary) or a different one (`switch-to`),
and in the case they are different the prefill is forced.
@fluca1978
Copy link
Collaborator Author

I've crated #226 as a possible implementation of your suggestion.
One thing I don't understand is why pgagroal_flush_server accepts a unsigned char as a server index, while other parts like pgagroal_get_primary handles an integer.
I've inserted a check in the extraction of the new primary, that could be avoided since if pgagroal_get_primary fails we don't need to prefill in any case, but the code looks stronger.

@jesperpedersen
Copy link
Collaborator

The pgagroal_get_server uses an integer as its out parameter to highlight that the value can be negative if the function fails to find a primary.

@fluca1978
Copy link
Collaborator Author

Got it, thanks.
I'm closing this same PR since #226 looks more what you have in mind.

@fluca1978 fluca1978 closed this May 4, 2022
fluca1978 added a commit to fluca1978/pgagroal that referenced this pull request May 5, 2022
Whenever the primary host is changed, by means of an explicit
`switch-to` command or by a primary failure, the connection flushing
is activated. If possible, the prefill should be also restored on the
new server.
As suggested in
<agroal#225 (comment)>
it would be nice to check if the specified new server is the same as
the old one (failure of the primary) or a different one (`switch-to`),
and in the case they are different the prefill is forced.

The prefill will always be to the `INITIAL` size when needed.
If the primary is not set, there is no need to prefill
with the `MIN_SIZE` value.

Close agroal#224
fluca1978 added a commit to fluca1978/pgagroal that referenced this pull request May 5, 2022
Whenever the primary host is changed, by means of an explicit
`switch-to` command or by a primary failure, the connection flushing
is activated. If possible, the prefill should be also restored on the
new server.
As suggested in
<agroal#225 (comment)>
it would be nice to check if the specified new server is the same as
the old one (failure of the primary) or a different one (`switch-to`),
and in the case they are different the prefill is forced.

The prefill will always be to the `INITIAL` size when needed.
If the primary is not set, there is no need to prefill
with the `MIN_SIZE` value.

Close agroal#224
jesperpedersen pushed a commit that referenced this pull request May 6, 2022
Whenever the primary host is changed, by means of an explicit
`switch-to` command or by a primary failure, the connection flushing
is activated. If possible, the prefill should be also restored on the
new server.
As suggested in
<#225 (comment)>
it would be nice to check if the specified new server is the same as
the old one (failure of the primary) or a different one (`switch-to`),
and in the case they are different the prefill is forced.

The prefill will always be to the `INITIAL` size when needed.
If the primary is not set, there is no need to prefill
with the `MIN_SIZE` value.

Close #224
jesperpedersen pushed a commit that referenced this pull request May 6, 2022
Whenever the primary host is changed, by means of an explicit
`switch-to` command or by a primary failure, the connection flushing
is activated. If possible, the prefill should be also restored on the
new server.
As suggested in
<#225 (comment)>
it would be nice to check if the specified new server is the same as
the old one (failure of the primary) or a different one (`switch-to`),
and in the case they are different the prefill is forced.

The prefill will always be to the `INITIAL` size when needed.
If the primary is not set, there is no need to prefill
with the `MIN_SIZE` value.

Close #224
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

prefill after switch-to
2 participants