SQL Backend

[jimbishopp]

This PR implements a new backend supporting PostgreSQL and CockroachDB SQL database platforms.

Implements #10253 without cloud connectivity.

[Tener]

Preliminary review, I need to dig deeper into backend stuff to properly review that.

[fspmarshall]

Disclaimer: I have had no prior experience with SQL. I did a small amount of background reading on SQL concurrency in order to review this PR, but take everything with a grain of salt.

[jimbishopp]

Latest changes include an optimization for the DeleteItems query.

Previous query:

teleport/lib/backend/postgres/tx.go

Lines 133 to 137 in 1abf0b6

	DELETE FROM item WHERE (key, id) NOT IN (
	SELECT key, id FROM lease WHERE (expires IS NULL OR expires > $1)
	UNION
	SELECT key, id FROM event
	)`

New query:

teleport/lib/backend/postgres/tx.go

Lines 134 to 141 in ce456d8

	DELETE FROM item WHERE (key, id) IN (
	SELECT key, id
	FROM item
	LEFT JOIN lease USING (key, id)
	LEFT JOIN event USING (key, id)
	WHERE event.key IS NULL
	AND (lease.key IS NULL OR lease.expires < $1)
	)`

Performance improved from 7 minutes to 76 ms on a 300K record dataset (100K per table).

OLD QUERY
teleport=# explain analyze select key,id from item where (key,id) not in (
        select key,id from lease where (expires is null or expires > now())
        union
        select key,id from event
);
                                                                         QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------------------------
 Seq Scan on item  (cost=14489.54..114117475.54 rows=50000 width=19) (actual time=422813.252..422813.254 rows=0 loops=1)
   Filter: (NOT (SubPlan 1))
   Rows Removed by Filter: 100000
   SubPlan 1
     ->  Materialize  (cost=14489.54..16521.56 rows=100001 width=40) (actual time=0.002..1.936 rows=50000 loops=100000)
           ->  Unique  (cost=14489.54..15239.55 rows=100001 width=40) (actual time=90.174..105.293 rows=100000 loops=1)
                 ->  Sort  (cost=14489.54..14739.55 rows=100001 width=40) (actual time=90.173..96.160 rows=100000 loops=1)
                       Sort Key: lease.key, lease.id
                       Sort Method: external merge  Disk: 3336kB
                       ->  Append  (cost=8.60..3447.63 rows=100001 width=40) (actual time=0.038..27.681 rows=100000 loops=1)
                             ->  Bitmap Heap Scan on lease  (cost=8.60..12.62 rows=1 width=19) (actual time=0.027..0.028 rows=0 loops=1)
                                   Recheck Cond: ((expires IS NULL) OR (expires > now()))
                                   ->  BitmapOr  (cost=8.60..8.60 rows=1 width=0) (actual time=0.025..0.025 rows=0 loops=1)
                                         ->  Bitmap Index Scan on lease_expires  (cost=0.00..4.30 rows=1 width=0) (actual time=0.020..0.020 rows=0 loops=1)
                                               Index Cond: (expires IS NULL)
                                         ->  Bitmap Index Scan on lease_expires  (cost=0.00..4.30 rows=1 width=0) (actual time=0.002..0.002 rows=0 loops=1)
                                               Index Cond: (expires > now())
                             ->  Seq Scan on event  (cost=0.00..1935.00 rows=100000 width=19) (actual time=0.009..19.800 rows=100000 loops=1)
 Planning Time: 0.413 ms
 Execution Time: 422814.115 ms

NEW QUERY
teleport=# explain analyze select key,id from item i left join lease l using (key,id) left join event e using (key,id) where e.key is null and (l.key is null or l.expires < now());
                                                            QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------------
 Nested Loop Left Join  (cost=4021.42..8265.51 rows=1 width=19) (actual time=76.165..76.167 rows=0 loops=1)
   Filter: ((l.key IS NULL) OR (l.expires < now()))
   ->  Hash Anti Join  (cost=4021.00..8265.00 rows=1 width=19) (actual time=76.165..76.166 rows=0 loops=1)
         Hash Cond: ((i.key = e.key) AND (i.id = e.id))
         ->  Seq Scan on item i  (cost=0.00..1736.00 rows=100000 width=19) (actual time=0.025..8.708 rows=100000 loops=1)
         ->  Hash  (cost=1935.00..1935.00 rows=100000 width=19) (actual time=38.984..38.984 rows=100000 loops=1)
               Buckets: 65536  Batches: 2  Memory Usage: 3234kB
               ->  Seq Scan on event e  (cost=0.00..1935.00 rows=100000 width=19) (actual time=0.009..14.667 rows=100000 loops=1)
   ->  Index Scan using lease_pk on lease l  (cost=0.42..0.49 rows=1 width=27) (never executed)
         Index Cond: (key = i.key)
         Filter: (i.id = id)
 Planning Time: 0.875 ms
 Execution Time: 76.285 ms

[r0mant]

@smallinsky @Tener Could you please give this another look?

[smallinsky]

I have little prior experience with SQL but the code LGTM.

[Tener]

As far as I can tell it looks good.

One thought that I have is that given the multitude of backends that we have we could benefit from having a comprehensive backend test suite testing backend correctness and performance. Preferably an automated or semi-automated one.

[jimbishopp]

@Tener

we could benefit from having a comprehensive backend test suite testing backend correctness and performance

We do! The test suite was very helpful when putting this together, especially by validating the correct error is returned from specific backend functions. I was able to iterate very quickly in the end because of this.

teleport/lib/backend/sqlbk/test.go

Line 70 in 3175fb1

test.RunBackendComplianceSuite(t, newBackend)

teleport/lib/backend/test/suite.go

Lines 105 to 112 in 3175fb1

	// RunBackendComplianceSuite runs the entire backend compliance suite,
	// creating a collection of named subtests under the context provided
	// by `t`.
	//
	// As each test requires a new backend instance it will invoke the supplied
	// `newBackend` function, which callers will use inject instances of the
	// backend under test.
	func RunBackendComplianceSuite(t *testing.T, newBackend Constructor) {

[Tener]

@jimbishopp

We do! The test suite was very helpful when putting this together, especially by validating the correct error is returned from specific backend functions. I was able to iterate very quickly in the end because of this.

This is good stuff, thanks for pointing that out. We could go bigger still with the tests to get a sense of the relative speed of individual backends under production-like loads... but this is probably just my inner benchmark junkie speaking 😉