fix(3088): disable pgsql endpoint by default

[lsytj0413]

Fix: https://github.com/restatedev/restate/issues/3088

when configuration it in config.toml, a log was printed：

Using the deprecated config option 'admin.query-enging.pgsql-bind-address'.

[AhmedSoliman]

What I suggest is to add an option to disable this port and we set it as disabled by default. I'm not sure if we'll go all in on removing/deprecating it before we are sure that it's not valuable to the community.

[lsytj0413]

What I suggest is to add an option to disable this port and we set it as disabled by default. I'm not sure if we'll go all in on removing/deprecating it before we are sure that it's not valuable to the community.

Is there any functionality that PostgreSQL queries can achieve but DataFusion queries cannot?

[AhmedSoliman]

Not really. It's mainly a convenience to use any postgres client to query instead restate CLI or the admin http endpoint.

[lsytj0413]

Not really. It's mainly a convenience to use any postgres client to query instead restate CLI or the admin http endpoint.

Ok， @tillrohrmann says there is an discussion about how to implement this, please let me known if the solution is finalized.

[lsytj0413]

Sorry for getting back to you so late @lsytj0413. The 1.3.0 release took more of my capacity than I hoped. I've given your PR a review and I think it does not fully allow to disable the psql endpoint yet. I think we need to make

https://github.com/restatedev/restate/blob/84e6e0f3d3c732df75cf66094d4cc20019c2d90a/crates/worker/src/lib.rs#L223

conditional on whether a bind address was configured or not. What's a pity is that I didn't manage to give you feedback quickly enough to include it in the 1.3.0 release. Changing the default behavior in a bug fix release is not great. Maybe we could argue that this was an oversight and it should have been shipped with 1.3.0?

In this implementation, I only print a warning log when the user explicitly sets this parameter, and use the default configuration when the user doesn't set it. This indeed doesn't disable the function. I think we have the following ways to disable this function:

1. keep the system's behavior consistent with the previous version. Only output a warning log to prompt the user when they use this feature, and officially discontinue this feature in the next version (after fully notifying the users).
2. modify the system behavior to disable the function when the user does not explicitly enable it. If the user explicitly enables it, then enable the function and output a warning log for prompting.

This implementation adopts the first solution (because it will keep the behavior consistent when the user upgrades the system). May I ask which solution do you think is more appropriate? Or are there any other better solutions?

[tillrohrmann]

I think you are right that we shouldn't change the system's behavior in a bug fix release. Technically we are printing a deprecation warning if the user opens a connection with the pgsql client so to some extent we have this covered. Then I would lean towards disabling the pgsql endpoint by default with the next minor release (1.4.0). This would then also mean that we need to wait with merging this PR until we no longer want to release bug fix releases for 1.3.

[lsytj0413]

I think you are right that we shouldn't change the system's behavior in a bug fix release. Technically we are printing a deprecation warning if the user opens a connection with the pgsql client so to some extent we have this covered. Then I would lean towards disabling the pgsql endpoint by default with the next minor release (1.4.0). This would then also mean that we need to wait with merging this PR until we no longer want to release bug fix releases for 1.3.

I'd like to confirm what to do next (Please forgive me if this is too verbose. My English is not yet very proficient.)

1. v1.3.x：we only printing a deprecation warning when server is starting and the user opens a connection with the pgsql client
2. v1.4.0：disable pgsql endpoint by defaults, and the user can explicitly enable it
3. v1.5.0：remove pgsql endpoint support

Is my understanding correct?

[tillrohrmann]

1. v1.3.x：we only printing a deprecation warning when server is starting and the user opens a connection with the pgsql client
2. v1.4.0：disable pgsql endpoint by defaults, and the user can explicitly enable it
3. v1.5.0：remove pgsql endpoint support

Is my understanding correct?

Yes, that sounds like a good plan.

[lsytj0413]

@tillrohrmann I have change the implementation, it will print an warning when server is starting, ptal

[tillrohrmann]

Thanks for updating the PR @lsytj0413. Since we are already printing a deprecation warning when connecting to the pqsql endpoint, I think we are actually covered and don't need to add more. I think that my previous statements were a bit confusing in this regard. Sorry for that. The next thing we should do is to disable the pgsql endpoint for the 1.4.0 release.

[lsytj0413]

Thanks for updating the PR @lsytj0413. Since we are already printing a deprecation warning when connecting to the pqsql endpoint, I think we are actually covered and don't need to add more. I think that my previous statements were a bit confusing in this regard. Sorry for that. The next thing we should do is to disable the pgsql endpoint for the 1.4.0 release.

I have change this implementation to step 2 - disable pgsql endpoint by defaults, ptal