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 replication specific functionality #12

Closed
wants to merge 13 commits into from

Conversation

petrosagg
Copy link

This PR brings our fork at the same state as the upstream rust-postgres PR for anything that is related to replication. Specifically this PR makes rust-postgres only offer support for a generic CopyBoth query mode and no additional support for the replication protocol. This brings in bug fixes that have existed in the upstream PR version of the code that have been missing from here

The replication protocol specific bits have been moved to mz_postgres_util in this PR MaterializeInc/materialize#17114

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ petrosagg
❌ jeff-davis
You have signed the CLA already but the status is still pending? Let us recheck it.

@petrosagg petrosagg changed the title remove replication specific functionlaity remove replication specific functionality Jan 13, 2023
@Venryx
Copy link

Venryx commented Jan 16, 2023

Just wanted to mention that: For several months I have been relying on the petrosagg "copy-both" branch for logical-replication support: https://github.com/petrosagg/rust-postgres/tree/copy-both

Earlier today I tried to do a fresh build, and got a compile error: object not found - no match for id (df8b5986088d29f29a3afb71eec5877171278387); class=Odb (9); code=NotFound (-3)

I cleared the docker cache and such, then realized the issue may have been caused by a force-push to petrosagg's copy-both branch, which then changed the sha-1 signatures of the commits. On checking the Github repo, it appears that was the case.

I attempted to resolve by trying the latest version of the "copy-both" branch, and got this error:

       →    Compiling postgres-protocol v0.6.4 (https://github.com/petrosagg/rust-postgres.git?rev=2fed91d76fd16bb75b7e3ee16df241b8ada438ed#2fed91d7)
       →    Compiling postgres-types v0.2.4 (https://github.com/petrosagg/rust-postgres.git?rev=2fed91d76fd16bb75b7e3ee16df241b8ada438ed#2fed91d7)
       →    Compiling tokio-postgres v0.7.7 (https://github.com/petrosagg/rust-postgres.git?rev=2fed91d76fd16bb75b7e3ee16df241b8ada438ed#2fed91d7)
       → error[E0599]: no method named `with_interval` found for struct `TcpKeepalive` in the current scope
       →   --> /usr/local/cargo/git/checkouts/rust-postgres-ff37caa29a1ecafb/2fed91d/tokio-postgres/src/keepalive.rs:17:43
       →    |
       → 17 |             tcp_keepalive = tcp_keepalive.with_interval(interval);
       →    |                                           ^^^^^^^^^^^^^ method not found in `TcpKeepalive`
       → 
       → error[E0599]: no method named `with_retries` found for struct `TcpKeepalive` in the current scope
       →   --> /usr/local/cargo/git/checkouts/rust-postgres-ff37caa29a1ecafb/2fed91d/tokio-postgres/src/keepalive.rs:22:43
       →    |
       → 22 |             tcp_keepalive = tcp_keepalive.with_retries(retries);
       →    |                                           ^^^^^^^^^^^^ help: there is a method with a similar name: `with_time`
       → 
       → For more information about this error, try `rustc --explain E0599`.
       → error: could not compile `tokio-postgres` due to 2 previous errors

I then tried various other commits, but was still hitting compile errors (I eventually went back far enough to where the copy_both_simple function no longer existed, which was obviously a non-starter, since that's why I needed the fork to begin with).

I then tried various commits from the MaterializeInc/rust-postgres fork (this repo), with the same results (the error shown above, or the copy_both_simple function missing if I went back too far).

I eventually remembered that I had made a fork of the petrosagg fork previously, as a backup plan for exactly this sort of situation. After switching to it, my project's build worked again as expected. (so it does seem to be from the force-pushed changes, rather than a caching issue)

Maybe this is already a known issue, but I figured I would mention it. (if for no other reason, to help other people hitting this issue and searching on it)

@petrosagg
Copy link
Author

superseded by #25

@petrosagg petrosagg closed this Aug 21, 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