Delete bgp-config endpoint contains a full table scan

[augustuswm]

We attempted to run a BGP config delete on the colo rack (running v7) and ran encountered the following 500:

00:10:01.575Z INFO 1cfdb5b6-e568-436a-a85f-7fecf1b8eef2 (dropshot_external_techport): request completed
    error_message_external = Internal Server Error
    error_message_internal = unexpected database error: query `SELECT COUNT(*) FROM "switch_port_settings_bgp_peer_config" WHERE ("switch_port_settings_bgp_peer_config"."bgp_config_id" = $1)` contains a full table/index scan which is explicitly disallowed
    file = /home/build/.cargo/git/checkouts/dropshot-a4a923d29dccc492/29ae98d/dropshot/src/server.rs:837
    latency_us = 37389
    local_addr = [fd00:1122:3344:10e::3]:12228
    method = DELETE
    remote_addr = [fd00:1122:3344:11f::2]:55422
    req_id = 9948d5fb-0019-496a-8ed4-da263f226f08
    response_code = 500
    uri = //v1/system/networking/bgp?name_or_id=6cd5ea03-2a64-4f9a-b469-c337c44795a6

This count query looks to be used to check if a BGP configuration is currently in use prior to deletion.

[david-crespo]

This could be fixed by adding an index, but this probably doesn't need to be a count either. We can use an exists query, though that might still need the index.

We might be able to simply attempt the update with an on_conflict to trigger the error if there already is something there. Edit: I see the count and update are about two different tables, so the on_conflict thing probably won't do it.

https://github.com/oxidecomputer/omicron/blob/baf1466214afaf77a8b9f9a669dbe18fa587ade7/nexus/db-queries/src/db/datastore/bgp.rs#L149-L164

[internet-diglett]

Did some verification in an a4x2 environment. It appears the most straightforward solve for this is to add an index, since we're not calling filter on the table's primary key:

root@[fd00:1122:3344:101::3]:32221/omicron> SHOW CREATE switch_port_settings_bgp_peer_config;
               table_name              |                                                     create_statement
---------------------------------------+----------------------------------------------------------------------------------------------------------------------------
  switch_port_settings_bgp_peer_config | CREATE TABLE public.switch_port_settings_bgp_peer_config (
                                       |     port_settings_id UUID NOT NULL,
                                       |     bgp_config_id UUID NOT NULL,
                                       |     interface_name STRING NOT NULL,
                                       |     addr INET NOT NULL,
                                       |     hold_time INT8 NULL,
                                       |     idle_hold_time INT8 NULL,
                                       |     delay_open INT8 NULL,
                                       |     connect_retry INT8 NULL,
                                       |     keepalive INT8 NULL,
                                       |     CONSTRAINT switch_port_settings_bgp_peer_config_pkey PRIMARY KEY (port_settings_id ASC, interface_name ASC, addr ASC)
                                       | )
(1 row)

Also, for observability purposes in the future, it's not unreasonable for us to want to filter for any bgp_peer_config that is associated with a specific bgp_config_id, so this index may have ended up getting added sooner or later anyway.

Without Index

root@[fd00:1122:3344:101::3]:32221/omicron> EXPLAIN SELECT COUNT(*) FROM "switch_port_settings_bgp_peer_config" WHERE ("switch_port_settings_bgp_peer_config"."bgp_con
fig_id" = '02641c1f-4b04-485d-b9fb-ee3945ee022f');
                                               info
---------------------------------------------------------------------------------------------------
  distribution: local
  vectorized: true

  • group (scalar)
  │ estimated row count: 1
  │
  └── • filter
      │ estimated row count: 1
      │ filter: bgp_config_id = '02641c1f-4b04-485d-b9fb-ee3945ee022f'
      │
      └── • scan
            estimated row count: 1 (100% of the table; stats collected 57 minutes ago)
            table: switch_port_settings_bgp_peer_config@switch_port_settings_bgp_peer_config_pkey
            spans: FULL SCAN

  index recommendations: 1
  1. type: index creation
     SQL command: CREATE INDEX ON switch_port_settings_bgp_peer_config (bgp_config_id);
(18 rows)

Time: 2ms total (execution 2ms / network 1ms)

With Index

root@[fd00:1122:3344:101::3]:32221/omicron> CREATE INDEX ON switch_port_settings_bgp_peer_config (bgp_config_id);
CREATE INDEX

Time: 1.354s total (execution 1.354s / network 0.000s)

root@[fd00:1122:3344:101::3]:32221/omicron>
root@[fd00:1122:3344:101::3]:32221/omicron>
root@[fd00:1122:3344:101::3]:32221/omicron> EXPLAIN SELECT COUNT(*) FROM "switch_port_settings_bgp_peer_config" WHERE ("switch_port_settings_bgp_peer_config"."bgp_config_id" = '02641c1f-4b04-485d-b9fb-ee3945ee022f');
                                                    info
------------------------------------------------------------------------------------------------------------
  distribution: local
  vectorized: true

  • group (scalar)
  │ estimated row count: 1
  │
  └── • scan
        estimated row count: 1 (100% of the table; stats collected 59 minutes ago)
        table: switch_port_settings_bgp_peer_config@switch_port_settings_bgp_peer_config_bgp_config_id_idx
        spans: [/'02641c1f-4b04-485d-b9fb-ee3945ee022f' - /'02641c1f-4b04-485d-b9fb-ee3945ee022f']
(10 rows)

Time: 2ms total (execution 1ms / network 0ms)

[internet-diglett]

I think I have a rough idea of how we could rewrite this as a CTE, but I don't think the juice is worth the squeeze at this point, and we're not going to have a lot of entries in these tables relative to our other tables, so I don't foresee the index being that costly in the grand scheme of things.