Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Startup SQL interactions with replication. #1576

Open
lmwnshn opened this issue May 8, 2021 · 6 comments
Open

Startup SQL interactions with replication. #1576

lmwnshn opened this issue May 8, 2021 · 6 comments
Labels
question/discussion Further information is requested.

Comments

@lmwnshn
Copy link
Contributor

lmwnshn commented May 8, 2021

Should startup SQL commands be replicated or not?

Currently, some SQL commands are just run on startup no matter what node you're on. Suppose the startup SQL included create table foo (a int), for example.

A. If the startup SQL is replicated, then the current implementation forces the following constraint: every replica must be brought up and ready before the primary is brought up. But this can be fixed (thanks @jrolli) if we take the time to implement a "give me my missing logs, my last log was X" feature.

B. If the startup SQL is not replicated, then weird things may happen if you refer to tables created by the startup process since the replica's RecoveryManager may be missing the oid mapping (haven't checked this).

I am inclined to say that we should go with A. But it is something to watch out for.

@lmwnshn lmwnshn added the question/discussion Further information is requested. label May 8, 2021
@jkosh44
Copy link
Contributor

jkosh44 commented May 9, 2021

It would be interesting/useful to know what the weird things that happen are and why they happen.
From a pure usability standpoint it would be nice to be able to start the servers in any order. However from an implementation standpoint it would probably be nice to treat the startup transactions the same as every other transaction and replicate them just the same.

This is also probably loosely related to the discussion at #1536. It might make sense to have recovery and replication be consistent with how they treat startup SQL, whether to read it from WAL/primary or just redo it themselves.

@jrolli
Copy link
Contributor

jrolli commented May 13, 2021

I'm not sure I agree with the ordering assertion in A. Any replica will need access to all logs since the last checkpoint (or database init) as part of coming online.

I think the larger question of "startup SQL commands" are what functionality they are doing. If these are things like bootstrapping the catalog on creating a database, then I think B sets us up for more pain whenever we incorporate checkpointing (since we need to avoid bootstrapping there as well).

@lmwnshn
Copy link
Contributor Author

lmwnshn commented May 14, 2021

@jkosh44 Here's an example of how you can get weird things to happen:

start primary
start replica1
modify startup.sql to switch create table order around, e.g., move the creation of noisepage_forecast_frequencies up by one statement
start replica2
insert into noisepage_forecast_frequencies on the primary
replica2 will segfault or insert into the wrong table

In general right now, it will appear to work, in what I would argue is mostly a happy accident / bugprone overall.
It works because if you run the exact same SQL commands at startup on the primary and the replicas, in the same order, and the task manager executes those in the same order, then in this case the tables will have the same OIDs.
So your message that goes over saying "hey can you do this to table OID xyz", is accidentally applied to the right table. Normally, the replica maintains a map of "primary oid -> local oid".

@lmwnshn
Copy link
Contributor Author

lmwnshn commented May 14, 2021

I think the larger question of "startup SQL commands" are what functionality they are doing.

@jrolli Good point. Right now, all the startup SQL is for bootstrapping internal tables for self-driving stuff. By the above example, IMO we want to replicate these so that the replica has a true sense of "yup this is definitely the table I want to apply the insert or whatever to".

We don't have a "give me all my missing logs" functionality right now; you're right that this would remove the ordering constraint. Updated to note that this is an implementation issue and not a design issue.

@jrolli
Copy link
Contributor

jrolli commented May 14, 2021

@lmwnshn, looking at the code, the current startup.sql flow appears to violate all sorts of assumptions (I'm surprised it actually works at all). Most notably it appears we are creating tables via SQL that appear to live outside of a database (i.e., they are global to the process) which raises questions about ownership, management, binding, etc.

Those concerns aside, though, I'm not convinced we want to replicate these tables since they appear to be tracking instance-specific information which may vary substantially between the primary and replicas (particularly if we allow replicas to answer read-only queries).

@lmwnshn
Copy link
Contributor Author

lmwnshn commented May 14, 2021

Another good point (on whether internal tables should be replicated). Hmm. I'll poke people on Slack.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question/discussion Further information is requested.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants