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

pglogical: SQL "frontend" related changes meta issue #105130

Open
12 of 35 tasks
otan opened this issue Jun 19, 2023 · 2 comments
Open
12 of 35 tasks

pglogical: SQL "frontend" related changes meta issue #105130

otan opened this issue Jun 19, 2023 · 2 comments
Labels
A-cdc Change Data Capture C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) meta-issue Contains a list of several other issues. T-cdc

Comments

@otan
Copy link
Contributor

otan commented Jun 19, 2023


x-ref #93404 for prototype PR
DMS meta issue #84505
Epic: https://cockroachlabs.atlassian.net/browse/CRDB-26486
test with https://github.com/jackc/pglogrepl

Jira issue: CRDB-28886

Epic CRDB-26486

@otan otan added the meta-issue Contains a list of several other issues. label Jun 19, 2023
@otan otan self-assigned this Jun 19, 2023
@cockroachdb cockroachdb deleted a comment from blathers-crl bot Jun 19, 2023
@otan otan added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Jun 19, 2023
@blathers-crl
Copy link

blathers-crl bot commented Jun 19, 2023

cc @cockroachdb/cdc

@blathers-crl blathers-crl bot added the A-cdc Change Data Capture label Jun 19, 2023
craig bot pushed a commit that referenced this issue Jun 26, 2023
105326: sql: implement pg_lsn operations r=zhouxing19 a=otan

## sql: add pg_lsn +- decimal operations

Release note (sql change): Add the ability to add numeric values to
LSNs, or sub a decimal value from a LSN.

## tree: implement lsn - lsn operation

Release note (sql change): Implement the `pg_lsn - pg_lsn = decimal`
builtin, which subtracts 2 LSNs to return a decimal.

----

Informs #105130



Co-authored-by: Oliver Tan <[email protected]>
craig bot pushed a commit that referenced this issue Jun 26, 2023
105391: colinfo: add version gate for storing pg_lsn types r=rafiss a=otan

We don't want mixed version clusters storing pg_lsn types, in case they need to rollback / older versions do not understand the type.

Informs #105130

Release note: None

105444: systemschema: stop running PostDeserializationChanges when building tables r=Xiang-Gu a=rafiss

### systemschema: add JobInfo to MakeSystemTables helper

This table was missing from the list, and the expected count of tables
was off since we weren't accounting for non-system tenant tables.

The function is only used for testing, so there was no impact.

---

### systemschema: stop running PostDeserializationChanges when building tables

We would like to remove old PostDeserializationChanges that are no
longer needed. In order to do so, we need to stop relying on them to
build system tables.

Instead, now we adjust the hard-coded system table descriptors and the
related helpers so that they create valid descriptors. This needed two
changes:
- Update the ConstraintID for check constraints.
- Update the primary index encoding so that it includes stored columns.

---

Epic: None
Release note: None

105476: kv: fix data race when updating pending txn in txnStatusCache r=arulajmani a=nvanbenschoten

Fixes #105244.

This commit avoids a data race by treating *roachpb.Transaction objects as immutable, and simply choosing the right transaction to keep in the cache when there is a choice to be made.

The behavior of this logic is tested by `TestTxnCacheUpdatesTxn`.

Release note: None

105480: kv: fix data race during retry of EndTxn after refresh r=arulajmani a=nvanbenschoten

Fixes #103687.
Fixes #103247.
Fixes #104791.

This commit avoids a data race between `splitEndTxnAndRetrySend` and `raceTransport` by avoiding a mutation of a shared `RequestUnion_EndTxn` object within an unshared `RequestUnion` object. The `raceTransport` makes an effort to copy the `BatchRequest`'s `RequestUnion` slice, but it does not copy the inner interface, so we can't play tricks to avoid a reallocation of the `RequestUnion_EndTxn`.

The commit also addresses a similar problem in
`retryTxnCommitAfterFailedParallelCommit`.

We may be able to fix this in the `raceTransport`, but doing so would require some reflection magic and this is currently failing CI, so we make the easier change.

Release note: None

105515: pgwire: fix race in TestConn r=knz a=rafiss

fixes #105410

A recent refactor introduced this race, since the context is used by two testing goroutines.

Release note: None

Co-authored-by: Oliver Tan <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
craig bot pushed a commit that referenced this issue Jun 26, 2023
105433: flowinfra: remove flowID from the "flow registry is draining" error r=yuzefovich a=yuzefovich

This doesn't seem that useful and unique values make it harder to aggregate this type of error.

Epic: None

Release note: None

105548: ccl/serverccl/diagnosticsccl: skip TestTenantReport r=tbg a=herkolategan

Refs: #101622

Reason: flaky test

Generated by bin/skip-test.

Release justification: non-production code changes

Release note: None
Epic: None

105554: sql: re-add encoding test for pg_lsn r=rafiss a=otan

I committed the json but not the binary test. Woops!

Informs: #105130

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Herko Lategan <[email protected]>
Co-authored-by: Oliver Tan <[email protected]>
@shermanCRL
Copy link
Contributor

We might consider contributing to or forking https://github.com/jackc/pglogrepl for things that are “only about the protocol”, and are not CRDB-specific.

otan added a commit to otan-cockroach/cockroach that referenced this issue Jun 26, 2023
craig bot pushed a commit that referenced this issue Jun 27, 2023
105575: sql: version gate casting to pg_lsn r=rafiss a=otan

Informs #105130

Release note: None

Co-authored-by: Oliver Tan <[email protected]>
craig bot pushed a commit that referenced this issue Jul 3, 2023
105400: pgrepl: allow logging in with `replication` parameter r=rafiss a=otan

This commit adds the `replication=database` connection parameter, which enables certain replication commands to be run.
See: https://www.postgresql.org/docs/current/protocol-replication.html

We do this by making it a session variable instead of a parameter on the connection like PG does. This adds a hidden `replication` connection parameter to accomplish this.

Informs: #105130

Release note: None

Co-authored-by: Oliver Tan <[email protected]>
craig bot pushed a commit that referenced this issue Jul 18, 2023
103837: ui: Network latency page improvements r=koorosh a=koorosh

- use connectivity endpoint as a source of connection statuses and latency;
- removed popup with disconnected peers, instead it will be shown
on latency matrix;

Resolves: #96101 

Release note (ui change): add "Show dead nodes" option
to show/hide dead/decommissioned nodes from latency matrix.

Release note (ui change): "No connections" popup menu is removed. Now failed
connections are displayed on the latencies matrix.

- [x] test coverage (storybook added for visual validation)

This PR depends on following PRs that should be merged before this one:
- #95429
- #99191

#### example 1. one node cannot establish connection to another one.
![Screenshot 2023-06-26 at 12 33 46](https://github.com/cockroachdb/cockroach/assets/3106437/2534e0fe-f5ba-48b5-8825-1c17a8112870)

#### example 2. Node 5 is stopped and it is considered as `unavailable` before setting as `dead`
![Screenshot 2023-06-26 at 10 52 20](https://github.com/cockroachdb/cockroach/assets/3106437/bd703d4c-9812-4061-9140-4a8f8d3a5da9)

#### example 3. Node 3 is dead. No connection from/to node. Show error message.
<img width="954" alt="Screenshot 2023-06-23 at 14 07 25" src="https://github.com/cockroachdb/cockroach/assets/3106437/8078c421-aeaa-4038-adf5-e3c69ba6d863">

#### example 4. Decommissioned node.
 
![Screenshot 2023-06-26 at 18 11 23](https://github.com/cockroachdb/cockroach/assets/3106437/dc4cb22e-34b7-4cd3-a391-b8ea5fd0232d)





106082: sql: add REPLICATION roleoption for replication mode r=rafiss a=otan

These commits add the REPLICATION roleoption (as per PG), and then uses it to authenticate whether a user can use the replication protocol.

Informs #105130

----

# sql: add REPLICATION roleoption

Matches PostgreSQL implementation of the REPLICATION roleoption.

Release note (sql change): Added the REPLICATION role option for a user,
which allows a user to use the streaming replication protocol.

# sql: only allow REPLICATION users to login with replication mode

In PG, the REPLICATION roleoption is required to use streaming
replication mode. Enforce the same constraint.

Release note: None



Co-authored-by: Andrii Vorobiov <[email protected]>
Co-authored-by: Oliver Tan <[email protected]>
@otan otan removed their assignment Jul 19, 2023
craig bot pushed a commit that referenced this issue Aug 8, 2023
106131: pgrepl: parse replication statements + handle IDENTIFY_SYSTEM r=cucaroach a=otan

Informs: #105130

## pgwire: ban extended protocol with REPLICATION protocol

In line with PostgreSQL, we prohibit the use of the extended protocol
when the replication protocol is in effect.

Release note: None

## pgrepl: implement IDENTIFY_SYSTEM command

This commit implements IDENTIFY_SYSTEM in the replication protocol,
which is used to retrieve metadata about replication state.

Most of this commit involves changing assumptions throughout the
codebase there is 1 parser - there is 2 if replication mode is enabled.
We've also had to change the parser to return `statements.Statement`
instead of `pgrepltree.ReplicationStatement` for compatibility with
`parser.Parse...`.

I've left the `LSN` fields as placeholders for now as we finalise on
what we'll eventually call it.

Release note: None

## sql: no-op SHOW SYNTAX in replication mode

Currently on the CLI, the command must parse on the server for it to
run. For replication related commands, we have not implemented `SHOW
SYNTAX` yet. This is a bit of effort and seems outside the scope of what
we want for now.

For now, handle replication commands by no-oping them.

Release note: None



Co-authored-by: Oliver Tan <[email protected]>
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Oct 4, 2023
This patch removes pg_lsn from the list of predefined types in
postgres that aren't yet supported, since it can now be parsed.

Informs cockroachdb#105130

Release note: None
DrewKimball added a commit to DrewKimball/cockroach that referenced this issue Oct 5, 2023
This patch removes pg_lsn from the list of predefined types in
postgres that aren't yet supported, since it can now be parsed.

Informs cockroachdb#105130

Release note: None
craig bot pushed a commit that referenced this issue Oct 5, 2023
111392: sql: add refcursor type r=DrewKimball a=DrewKimball

#### types: remove pg_lsn from postgresPredefinedTypeIssues

This patch removes pg_lsn from the list of predefined types in
postgres that aren't yet supported, since it can now be parsed.

Informs #105130

Release note: None

#### sql: check unsupported types during schema changes and type/function creation

It is necessary to check the cluster version when building an expression
that references a newly-added type, in order to ensure that all nodes
support that type. Previously, these checks were omitted for casts in
the vectorized engine, expressions for partial index predicates and
check constraints, function parameters and return types, and user-defined
composite types.

This patch adds version-checking for function/procedure parameters and
return types, as well as for user-defined types. It also augments the
type-checking logic for casts and type annotations with a version
check; this handles the remaining cases. The execution-time checks
for cast expressions are left untouched, just in case the new
type-checking logic misses important cases.

For now, these changes only apply to the `PG_LSN` type, which will
be new in 23.2. A future commit will add support for `REFCURSOR`, and
will need to use the same checks.

Informs #111560

Release note: None

#### sql: add support for REFCURSOR data type

This patch adds support for the `REFCURSOR` type. It is a special type
used to hold the name of a cursor for PLpgSQL, although it can be used
in other contexts (like function return types or column types). `REFCURSOR`
is a string under the hood, and behaves like the `TEXT` data type in most
cases.

`REFCURSOR` has no casts in the `pg_cast` table; instead, all of its
casts are the "IO" string casts. That means that there is an assignment
cast from every type to `REFCURSOR`, and an explicit cast from `REFCURSOR`
to every other type (except other string types, which are again assignment).
See `ContextOriginAutomaticIOConversion` in `cast.go` for more info.

This patch also adds `REFCURSOR` to the checks from the previous commit,
so that statements with `REFCURSOR` are disallowed unless all nodes
are running v23.2.

Fixes #111560

Release note (sql change): Added support for the `REFCURSOR` data type.
`REFCURSOR` is a special string type that is used to handle cursors.
PLpgSQL cursor declarations are required to use a variable of type
`REFCURSOR`, and the name of a cursor can be passed to and from a
PLpgSQL function or procedure.

Co-authored-by: Drew Kimball <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cdc Change Data Capture C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) meta-issue Contains a list of several other issues. T-cdc
Projects
None yet
Development

No branches or pull requests

2 participants