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

sql: add support for Hasura #72407

Closed
8 of 10 tasks
otan opened this issue Nov 3, 2021 · 15 comments
Closed
8 of 10 tasks

sql: add support for Hasura #72407

otan opened this issue Nov 3, 2021 · 15 comments
Labels
A-tools-hasura 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-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@otan
Copy link
Contributor

otan commented Nov 3, 2021

For anyone pursuing updates here, last I've heard is that Hasura is currently working towards making integrations with other databases easier. As soon as we can get any update, we'll look to get the ball moving on this again.

We know we have some features that we need to implement (and are looking to get a lot of these done in the next 6-12 months!) and we hope we can get this working for all of you soon!


Prototype branches

List of needed changes so far:

At present time, I have the Haskell engine up, but the pytest has UDFs all over the test suite. Impossible to get around - we need to implement them or the Hasura suite needs to not use them.

Epic CRDB-11916

Jira issue: CRDB-11915

@otan otan added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-tools-hasura C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) and removed C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. labels Nov 3, 2021
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Nov 3, 2021
@rafiss rafiss added the meta-issue Contains a list of several other issues. label Nov 4, 2021
@rafiss
Copy link
Collaborator

rafiss commented Nov 4, 2021

We do not support the ON CONFLICT ((expr)) clause, or the ON CONFLICT ... DO UPDATE clause.

Could you clarify the second half of that sentence? CRDB does have ON CONFLICT ... DO UPDATE: https://www.cockroachlabs.com/docs/stable/insert.html#on-conflict-clause

@otan
Copy link
Contributor Author

otan commented Nov 4, 2021

Huh we'll I had a syntax error when I tried just removing ((expr)). Maybe it's fine.

@rafiss
Copy link
Collaborator

rafiss commented Nov 4, 2021

This query finishes in 20mins on CRDB: https://gist.github.com/otan/498fc5fef15f6147be5ae1f429ca146f. Root cause is pg_catalog.col_description which is slow.

that is affecting activerecord too cockroachdb/activerecord-cockroachdb-adapter#234

One improvement is to resolve #69495 -- I think we should prioritize this since it's coming up for so many different ORMs.

@rafiss
Copy link
Collaborator

rafiss commented Nov 8, 2021

Here's an example of how to improve the introspection query so it doesn't use col_description: cockroachdb/activerecord-cockroachdb-adapter#235

Huh well I had a syntax error when I tried just removing ((expr)). Maybe it's fine.

It'd need to be changed to ON CONFLICT (constraint_name) DO UPDATE

otan added a commit to otan-cockroach/graphql-engine that referenced this issue Nov 8, 2021
CockroachDB doesn't support `index` as a table alias, but PostgreSQL
does. Since `idx` is supported by both, change the metadata query
accordingly.

Refs cockroachdb/cockroach#72407
otan added a commit to otan-cockroach/graphql-engine that referenced this issue Nov 8, 2021
CockroachDB doesn't support `index` as a table alias, but PostgreSQL
does. Since `idx` is supported by both, change the metadata query
accordingly.

Refs cockroachdb/cockroach#72407
@dbettin
Copy link

dbettin commented Dec 7, 2021

Is this a feature that is actively being worked on? Or is it a "maybe sometime in the future" type of feature?

@otan
Copy link
Contributor Author

otan commented Dec 7, 2021

Hi @dbettin! We've been trying to get in contact with Hasura as some of the changes will probably have to be on their side (I've even tried skirting the waters by submitting a PR: hasura/graphql-engine#7788). Unfortunately I haven't heard any updates recently.

There is also stuff we need to do on our side, which are already in the works.

@gunrein
Copy link

gunrein commented Dec 7, 2021

Hey @otan - thanks for raising the PR, this is great! I joined Hasura recently to lead product and was just introduced via email to Vy, Andy, and Mark from Cockroach earlier today so this is timely. We're eager to work with you on this effort. Right now we're working on an improvement that'll make it easier to collaborate with you on supporting Cockroach and we'd love to engage around that as soon as it is ready to try out. It is fairly research heavy at the moment so timelines are hard to predict, but sometime in the next couple of months is likely.

@exalate-issue-sync exalate-issue-sync bot removed 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. A-tools-hasura labels Dec 13, 2021
@rafiss rafiss added A-tools-hasura 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. labels Dec 13, 2021
@ghost
Copy link

ghost commented Jan 13, 2022

Hi @otan - My name is Dennis and I work with @gunrein on the product team here at Hasura. There is quite the buzz here regarding your recent merge.

We are very much looking forward to working more closely with you in an effort to holistically support CokcroachDB via the Hasura GraphQL service. At present the absence of a proper validation framework and existence of support documentation from Hasura specifically means we have some planning/research to do.

I will be following up via email to setup a more thorough call so we can get a better scope of how best we can support CockroachDB moving forward.

@vy-ton
Copy link
Contributor

vy-ton commented Jan 13, 2022

@dennislaviolette Thanks for the update. Please include [email protected] and [email protected]

@GavinRay97
Copy link

GavinRay97 commented Jan 28, 2022

The Hasura Postgres backend changes here could (I believe) be ported to a new Cockroach backend, by copy-pasting the entire Postgres backend code and then merging the changes here. That would be a start, at least.


In the event anyone wants to test this more easily in the future, here is a Docker Compose to set up Hasura + Cockroach.

You can use the Connect DB from Hasura web console at http://localhost:8080 to (try) to add a new Postgres DB from the COCKROACHDB_URL environment variable:

version: "3.6"

services:
    cockroach:
        image: cockroachdb/cockroach:v21.2.4
        ports:
            - "26257:26257"
            - "8070:8080"
        command: start-single-node --insecure
        volumes:
            - "${PWD}/cockroach-data/crdb:/cockroach/cockroach-data"

    postgres:
        image: postgres:14
        restart: always
        volumes:
            - db_data:/var/lib/postgresql/data
        environment:
            POSTGRES_PASSWORD: postgrespassword

    graphql-engine:
        image: hasura/graphql-engine:v2.1.1
        ports:
            - "8080:8080"
        depends_on:
            - "postgres"
        restart: always
        environment:
            COCKROACHDB_URL: postgresql://root@cockroach:26257/defaultdb?sslmode=disable
            HASURA_GRAPHQL_METADATA_DATABASE_URL: postgresql://postgres:postgrespassword@postgres:5432/postgres
            HASURA_GRAPHQL_DATABASE_URL: postgresql://postgres:postgrespassword@postgres:5432/postgres
            HASURA_GRAPHQL_ENABLE_CONSOLE: "true"
            HASURA_GRAPHQL_DEV_MODE: "true"
            HASURA_GRAPHQL_ENABLED_LOG_TYPES: startup, http-log, webhook-log, websocket-log, query-log

volumes:
    db_data:

image

@c-nv-s
Copy link

c-nv-s commented Mar 31, 2022

is there any update on the progress of this ticket? or is it a case of it has lost out to other more important things?

craig bot pushed a commit that referenced this issue Jun 30, 2022
79134: kv: support FOR {UPDATE,SHARE} SKIP LOCKED r=arulajmani a=nvanbenschoten

KV portion of #40476.
Assists #62734.
Assists #72407.
Assists #78564.

**NOTE: the SQL changes here were extracted from this PR and moved to #83627. This allows us to land the KV portion of this change without exposing it yet.**

```sql
CREATE TABLE kv (k INT PRIMARY KEY, v INT)
INSERT INTO kv VALUES (1, 1), (2, 2), (3, 3)


-- in session 1
BEGIN; UPDATE kv SET v = 0 WHERE k = 1 RETURNING *

  k | v
----+----
  1 | 0


-- in session 2
BEGIN; SELECT * FROM kv ORDER BY k LIMIT 1 FOR UPDATE SKIP LOCKED

  k | v
----+----
  2 | 2


-- in session 3
BEGIN; SELECT * FROM kv FOR UPDATE SKIP LOCKED

  k | v
----+----
  3 | 3
```

These semantics closely match those of FOR {UPDATE,SHARE} SKIP LOCKED in PostgreSQL. With SKIP LOCKED, any selected rows that cannot be immediately locked are skipped. Skipping locked rows provides an inconsistent view of the data, so this is not suitable for general purpose work, but can be used to avoid lock contention with multiple consumers accessing a queue-like table.

[Here](https://www.pgcasts.com/episodes/the-skip-locked-feature-in-postgres-9-5) is a short video that explains why users might want to use SKIP LOCKED in Postgres. The same motivation applies to CockroachDB. However, SKIP LOCKED is not a complete solution to queues, as MVCC garbage will still become a major problem with sufficiently high consumer throughput. Even with a very low gc.ttl, CockroachDB does not garbage collect MVCC garbage fast enough to avoid slowing down consumers that scan from the head of a queue over MVCC tombstones of previously consumed queue entries.

----

### Implementation

Skip locked has a number of touchpoints in Storage and KV. To understand these, we first need to understand the isolation model of skip-locked. When a request is using a SkipLocked wait policy, it behaves as if run at a weaker isolation level for any keys that it skips over. If the read request does not return a key, it does not make a claim about whether that key does or does not exist or what the key's value was at the read's MVCC timestamp. Instead, it only makes a claim about the set of keys that are returned. For those keys which were not skipped and were returned (and often locked, if combined with a locking strength, though this is not required), serializable isolation is enforced.

When the `pebbleMVCCScanner` is configured with the skipLocked option, it does not include locked keys in the result set. To support this, the MVCC layer needs to be provided access to the in-memory lock table, so that it can determine whether keys are locked with unreplicated lock. Replicated locks are represented as intents, which will be skipped over in getAndAdvance.

Requests using the SkipLocked wait policy acquire the same latches as before and wait on all latches ahead of them in line. However, if a request is using a SkipLocked wait policy, we always perform optimistic evaluation. In Replica.collectSpansRead, SkipLocked reads are able to constrain their read spans down to point reads on just those keys that were returned and were not already locked. This means that there is a good chance that some or all of the write latches that the SkipLocked read would have blocked on won't overlap with the keys that the request ends up returning, so they won't conflict when checking for optimistic conflicts.

Skip locked requests do not scan the lock table when initially sequencing. Instead, they capture a snapshot of the in-memory lock table while sequencing and scan the lock table as they perform their MVCC scan using the btree snapshot stored in the concurrency guard. MVCC was taught about skip locked in the previous commit.

Skip locked requests add point reads for each of the keys returned to the timestamp cache, instead of adding a single ranged read. This satisfies the weaker isolation level of skip locked. Because the issuing transaction is not intending to enforce serializable isolation across keys that were skipped by its request, it does not need to prevent writes below its read timestamp to keys that were skipped.

Similarly, Skip locked requests only records refresh spans for the individual keys returned, instead of recording a refresh span across the entire read span. Because the issuing transaction is not intending to enforce serializable isolation across keys that were skipped by its request, it does not need to validate that they have not changed if the transaction ever needs to refresh.

----

### Benchmarking

I haven't done any serious benchmarking with this SKIP LOCKED yet, though I'd like to. At some point, I would like to build a simple queue-like workload into the `workload` tool and experiment with various consumer access patterns (non-locking reads, locking reads, skip-locked reads), indexing schemes, concurrency levels (for producers and consumers), and batch sizes.

82915: sql: add locality to system.sql_instances table r=rharding6373 a=rharding6373

This PR adds the column `locality` to the `system.sql_instances` table
that contains the locality (e.g., region) of a SQL instance. The encoded
locality is a string representing the `roachpb.Locality` that may have
been provided when the instance was created.

This change also pipes the locality through `InstanceInfo`. This will
allow us to determine and use locality information of other SQL
instances, e.g. in DistSQL for multi-tenant locality-awareness
distribution planning.

Informs: #80678

Release note (sql change): Table `system.sql_instances` has a new
column, `locality`, that stores the locality of a SQL instance if it was
provided when the instance was started. This exposes a SQL instance's
locality to other instances in the cluster for query planning.


83418: loopvarcapture: do not flag `defer` within local closure r=srosenberg,dhartunian a=renatolabs

Previously, handling of `defer` statements in the `loopvarcapture`
linter was naive: whenever a `defer` statement in the body of a loop
referenced a loop variable, the linter would flag it as an invalid
reference. However, that can be overly restrictive, as a relatively
common idiom is to create literal functions and immediately call them
so as to take advantage of `defer` semantics, as in the example below:

```go
for _, n := range numbers {
    // ...
    func() {
           // ...
           defer func() { doSomewithing(n) }() // always safe
           // ...
    }()
}
```

The above reference is valid because it is guaranteed to be called
with the correct value for the loop variable.

A similar scenario occurs when a closure is assigned to a local
variable for use within the loop:

```go
for _, n := range numbers {
    // ...
    helper := func() {
           // ...
           defer func() { doSomething(n) }()
           // ...
    }
    // ...
    helper() // always safe
}
```

In the snippet above, calling the `helper` function is also always
safe because the `defer` statement is scoped to the closure containing
it. However, it is still *not* safe to call the helper function within
a Go routine.

This commit updates the `loopvarcapture` linter to recognize when a
`defer` statement is safe because it is contained in a local
closure. The two cases illustrated above will no longer be flagged,
allowing for that idiom to be used freely.

Release note: None.

83545: sql/schemachanger: move end to end testing to one test per-file r=fqazi a=fqazi

Previously, we allowed multiple tests per-file for end-to-end
testing inside the declarative schema changer. This was inadequate
because we plan on extending the end-to-end testing to start injecting
additional read/write operations at different stages, which would
make it difficult. To address this, this patch will split tests into
individual files, with one test per file. Additionally, it extends
support to allow multiple statements per-test statement, for transaction
support testing (this is currently unused).

Release note: None

Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: rharding6373 <[email protected]>
Co-authored-by: Renato Costa <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>
@jordanlewis
Copy link
Member

Take a look at this issue for some more progress updates: hasura/graphql-engine#8491

@dameleney
Copy link

dameleney commented Oct 28, 2022

Hasura is pleased to announce the beta release for CockroachDB datasource on the Hasura GraphQL Engine for Hasura CE, Cloud, and EE in v2.15. 🚀 🚀 🚀 Beta release for v2.15 is expected to take place Monday 10/31.

In this beta release Hasura supports Queries and Mutations with plans to add support for subscription before the GA release, which will coincide with the release of CockroachDB v22.2 (tentatively scheduled for early December).

CockroachDB is a distributed SQL database built on a transactional and strongly-consistent key-value store. It scales horizontally; survives disk, machine, rack, and even datacenter failures with minimal latency disruption and no manual intervention; supports strongly-consistent ACID transactions; and provides a familiar SQL API for structuring, manipulating, and querying data.

Together with Hasura, CockroachDB now supports: Instant GraphQL & REST APIs, Declarative Role Based Authorization, Advanced Security and Performance with rate limiting, allow lists, and caching.

@dameleney
Copy link

🎉💥 Excited to officially announce the beta release of CockroachDB on Hasura v2.15.0 currently available on cloud and oss versions of Hasura. Support for CockroachDB will be considered GA when the minimum required CockroachDB version, v22.2, which is currently in beta is released as GA and pushed to CockroachDB's managed solutions.

Find out more in this blog post or follow the instructions in the CockroachDB docs to get started building with Hasura and CockroachDB today.

Coming soon!!! We are hoping to release support for subscriptions for CockroachDB in the coming weeks.

@rafiss
Copy link
Collaborator

rafiss commented Feb 23, 2023

Since the initial support has been our for some time now, I'll close this. We'll track future work in other issues. (And to any watchers of this issue, please file issues if you encounter any problems.)

@rafiss rafiss closed this as completed Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tools-hasura 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-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

No branches or pull requests

9 participants