BGWORKER_BYPASS_ALLOWCONN used nowhere (infra part of on-line checksum switcher)
Hi all,
BGWORKER_BYPASS_ALLOWCONN has been added by commit eed1ce7, which is an
infrastructure piece to be able to enable and disable dynamically
checksums on a cluster. The main idea is to be able to bypass
datallowconn which allows a background worker to connect to a database
even if the database is set to refuse connections so as its checksums
can be calculated and updated.
At the end, the dynamic switch for checksums has been reverted as of
a228cc13, and a set of rather-used APIs have been changed for what looks
like no reason now:
- BackgroundWorkerInitializeConnection
- BackgroundWorkerInitializeConnectionByOid
- InitPostgres
So all background workers would not be able to compile because of that.
Would we want to drop this unused interface or keep it?
Even if this is not removed, bgworker.sgml needs to be updated with the
new definition of BackgroundWorkerInitializeConnection and
BackgroundWorkerInitializeConnectionByOid which are missing the third
argument "uint32 flags", as well as the description for
BGWORKER_BYPASS_ALLOWCONN. I can personally see more reasons to revert
that portion as well and consider it again for v12 or onwards if the
on-line checksum switch is proposed again.
Magnus, Daniel, what do you think?
Thanks,
--
Michael
On Sun, Apr 22, 2018 at 1:11 PM, Michael Paquier <michael@paquier.xyz>
wrote:
Hi all,
BGWORKER_BYPASS_ALLOWCONN has been added by commit eed1ce7, which is an
infrastructure piece to be able to enable and disable dynamically
checksums on a cluster. The main idea is to be able to bypass
datallowconn which allows a background worker to connect to a database
even if the database is set to refuse connections so as its checksums
can be calculated and updated.At the end, the dynamic switch for checksums has been reverted as of
a228cc13, and a set of rather-used APIs have been changed for what looks
like no reason now:
- BackgroundWorkerInitializeConnection
- BackgroundWorkerInitializeConnectionByOid
- InitPostgres
So all background workers would not be able to compile because of that.
Would we want to drop this unused interface or keep it?Even if this is not removed, bgworker.sgml needs to be updated with the
new definition of BackgroundWorkerInitializeConnection and
BackgroundWorkerInitializeConnectionByOid which are missing the third
argument "uint32 flags", as well as the description for
BGWORKER_BYPASS_ALLOWCONN. I can personally see more reasons to revert
that portion as well and consider it again for v12 or onwards if the
on-line checksum switch is proposed again.Magnus, Daniel, what do you think?
I think this feature is definitely worth keeping, regardless. It's useful
elsewhere, in externally maintained bgwriters.
The incorrect documentation is clearly a bug of course, and I will fix.
Good spot!
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
On 22 Apr 2018, at 14:04, Magnus Hagander <magnus@hagander.net> wrote:
On Sun, Apr 22, 2018 at 1:11 PM, Michael Paquier <michael@paquier.xyz <mailto:michael@paquier.xyz>> wrote:
Hi all,BGWORKER_BYPASS_ALLOWCONN has been added by commit eed1ce7, which is an
infrastructure piece to be able to enable and disable dynamically
checksums on a cluster. The main idea is to be able to bypass
datallowconn which allows a background worker to connect to a database
even if the database is set to refuse connections so as its checksums
can be calculated and updated.At the end, the dynamic switch for checksums has been reverted as of
a228cc13, and a set of rather-used APIs have been changed for what looks
like no reason now:
- BackgroundWorkerInitializeConnection
- BackgroundWorkerInitializeConnectionByOid
- InitPostgres
So all background workers would not be able to compile because of that.
Would we want to drop this unused interface or keep it?Even if this is not removed, bgworker.sgml needs to be updated with the
new definition of BackgroundWorkerInitializeConnection and
BackgroundWorkerInitializeConnectionByOid which are missing the third
argument "uint32 flags", as well as the description for
BGWORKER_BYPASS_ALLOWCONN. I can personally see more reasons to revert
that portion as well and consider it again for v12 or onwards if the
on-line checksum switch is proposed again.Magnus, Daniel, what do you think?
I think this feature is definitely worth keeping, regardless. It's useful elsewhere, in externally maintained bgwriters.
+1, I think this is worth keeping in for 11.
cheers ./daniel
On Sun, Apr 22, 2018 at 03:56:32PM +0200, Daniel Gustafsson wrote:
+1, I think this is worth keeping in for 11.
Okay, that's fine for me. Thanks for updating the documentation.
Perhaps it would be nicer for plugin developers to add a comment in
bgworker.h as well about what BGWORKER_BYPASS_ALLOWCONN allows to
achieve?
--
Michael
On Mon, Apr 23, 2018 at 9:25 AM, Michael Paquier <michael@paquier.xyz>
wrote:
On Sun, Apr 22, 2018 at 03:56:32PM +0200, Daniel Gustafsson wrote:
+1, I think this is worth keeping in for 11.
Okay, that's fine for me. Thanks for updating the documentation.
Perhaps it would be nicer for plugin developers to add a comment in
bgworker.h as well about what BGWORKER_BYPASS_ALLOWCONN allows to
achieve?
Yeah, that makes sense. Will add as well.
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>