From 88a9bd6d3c9126037c73dd797077122f18b95a34 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 10 Feb 2021 19:40:32 -0500 Subject: [PATCH] state: remove duplicate index on the checks table By using a new pattern for more specific indexes. This allows us to use the same index for both service checks and node checks. It removes the abstraction around memdb.Txn operations, and isolates all of the enterprise differences in a single place (the indexer). --- agent/consul/state/catalog.go | 20 +++++++-- agent/consul/state/catalog_oss.go | 37 ++++++++++++---- agent/consul/state/catalog_schema.go | 43 ++++--------------- .../testdata/TestStateStoreSchema.golden | 4 +- 4 files changed, 56 insertions(+), 48 deletions(-) diff --git a/agent/consul/state/catalog.go b/agent/consul/state/catalog.go index 25a4a8568..98650230f 100644 --- a/agent/consul/state/catalog.go +++ b/agent/consul/state/catalog.go @@ -1315,9 +1315,14 @@ func (s *Store) deleteServiceTxn(tx WriteTxn, idx uint64, nodeName, serviceID st return nil } + // TODO: accept a non-pointer value for EnterpriseMeta + if entMeta == nil { + entMeta = structs.DefaultEnterpriseMeta() + } // Delete any checks associated with the service. This will invalidate // sessions as necessary. - checks, err := catalogListServiceChecks(tx, nodeName, serviceID, entMeta) + q := NodeServiceQuery{Node: nodeName, Service: serviceID, EnterpriseMeta: *entMeta} + checks, err := tx.Get(tableChecks, indexNodeService, q) if err != nil { return fmt.Errorf("failed service check lookup: %s", err) } @@ -1766,6 +1771,13 @@ func (s *Store) deleteCheckCASTxn(tx WriteTxn, idx, cidx uint64, node string, ch return true, nil } +// NodeServiceQuery is a type used to query the checks table. +type NodeServiceQuery struct { + Node string + Service string + structs.EnterpriseMeta +} + // deleteCheckTxn is the inner method used to call a health // check deletion within an existing transaction. func (s *Store) deleteCheckTxn(tx WriteTxn, idx uint64, node string, checkID types.CheckID, entMeta *structs.EnterpriseMeta) error { @@ -2137,7 +2149,8 @@ func parseCheckServiceNodes( // First add the node-level checks. These always apply to any // service on the node. var checks structs.HealthChecks - iter, err := catalogListNodeChecks(tx, sn.Node) + q := NodeServiceQuery{Node: sn.Node, EnterpriseMeta: *structs.DefaultEnterpriseMeta()} + iter, err := tx.Get(tableChecks, indexNodeService, q) if err != nil { return 0, nil, err } @@ -2147,7 +2160,8 @@ func parseCheckServiceNodes( } // Now add the service-specific checks. - iter, err = catalogListServiceChecks(tx, sn.Node, sn.ServiceID, &sn.EnterpriseMeta) + q = NodeServiceQuery{Node: sn.Node, Service: sn.ServiceID, EnterpriseMeta: sn.EnterpriseMeta} + iter, err = tx.Get(tableChecks, indexNodeService, q) if err != nil { return 0, nil, err } diff --git a/agent/consul/state/catalog_oss.go b/agent/consul/state/catalog_oss.go index 04bd7515d..f0e59eeb6 100644 --- a/agent/consul/state/catalog_oss.go +++ b/agent/consul/state/catalog_oss.go @@ -4,6 +4,7 @@ package state import ( "fmt" + "strings" memdb "github.com/hashicorp/go-memdb" @@ -12,6 +13,34 @@ import ( func withEnterpriseSchema(_ *memdb.DBSchema) {} +func indexNodeServiceFromHealthCheck(raw interface{}) ([]byte, error) { + hc, ok := raw.(*structs.HealthCheck) + if !ok { + return nil, fmt.Errorf("unexpected type %T for structs.HealthCheck index", raw) + } + + if hc.Node == "" { + return nil, errMissingValueForIndex + } + + var b indexBuilder + b.String(strings.ToLower(hc.Node)) + b.String(strings.ToLower(hc.ServiceID)) + return b.Bytes(), nil +} + +func indexFromNodeServiceQuery(arg interface{}) ([]byte, error) { + hc, ok := arg.(NodeServiceQuery) + if !ok { + return nil, fmt.Errorf("unexpected type %T for NodeServiceQuery index", arg) + } + + var b indexBuilder + b.String(strings.ToLower(hc.Node)) + b.String(strings.ToLower(hc.Service)) + return b.Bytes(), nil +} + func serviceIndexName(name string, _ *structs.EnterpriseMeta) string { return fmt.Sprintf("service.%s", name) } @@ -156,14 +185,6 @@ func catalogListChecks(tx ReadTxn, _ *structs.EnterpriseMeta) (memdb.ResultItera return tx.Get("checks", "id") } -func catalogListNodeChecks(tx ReadTxn, node string) (memdb.ResultIterator, error) { - return tx.Get("checks", indexNodeServiceCheck, node, false) -} - -func catalogListServiceChecks(tx ReadTxn, node string, service string, _ *structs.EnterpriseMeta) (memdb.ResultIterator, error) { - return tx.Get("checks", indexNodeService, node, service) -} - func catalogInsertCheck(tx WriteTxn, chk *structs.HealthCheck, idx uint64) error { // Insert the check if err := tx.Insert("checks", chk); err != nil { diff --git a/agent/consul/state/catalog_schema.go b/agent/consul/state/catalog_schema.go index 6b5ebfbf1..4047df272 100644 --- a/agent/consul/state/catalog_schema.go +++ b/agent/consul/state/catalog_schema.go @@ -17,13 +17,12 @@ const ( tableGatewayServices = "gateway-services" tableMeshTopology = "mesh-topology" - indexID = "id" - indexServiceName = "service" - indexConnect = "connect" - indexKind = "kind" - indexStatus = "status" - indexNodeServiceCheck = "node_service_check" - indexNodeService = "node_service" + indexID = "id" + indexServiceName = "service" + indexConnect = "connect" + indexKind = "kind" + indexStatus = "status" + indexNodeService = "node_service" ) // nodesTableSchema returns a new table schema used for storing node @@ -170,37 +169,13 @@ func checksTableSchema() *memdb.TableSchema { Lowercase: true, }, }, - indexNodeServiceCheck: { - Name: indexNodeServiceCheck, - AllowMissing: true, - Unique: false, - Indexer: &memdb.CompoundIndex{ - Indexes: []memdb.Indexer{ - &memdb.StringFieldIndex{ - Field: "Node", - Lowercase: true, - }, - &memdb.FieldSetIndex{ - Field: "ServiceID", - }, - }, - }, - }, indexNodeService: { Name: indexNodeService, AllowMissing: true, Unique: false, - Indexer: &memdb.CompoundIndex{ - Indexes: []memdb.Indexer{ - &memdb.StringFieldIndex{ - Field: "Node", - Lowercase: true, - }, - &memdb.StringFieldIndex{ - Field: "ServiceID", - Lowercase: true, - }, - }, + Indexer: indexerSingle{ + fromArgsIndexer: indexFromSingleArg(indexFromNodeServiceQuery), + fromObjectIndexer: indexFromObject(indexNodeServiceFromHealthCheck), }, }, }, diff --git a/agent/consul/state/testdata/TestStateStoreSchema.golden b/agent/consul/state/testdata/TestStateStoreSchema.golden index 2b817d029..9b0abeadd 100644 --- a/agent/consul/state/testdata/TestStateStoreSchema.golden +++ b/agent/consul/state/testdata/TestStateStoreSchema.golden @@ -52,9 +52,7 @@ table=checks index=node allow-missing indexer=github.com/hashicorp/go-memdb.StringFieldIndex Field=Node Lowercase=true index=node_service allow-missing - indexer=github.com/hashicorp/go-memdb.CompoundIndex Indexes=[github.com/hashicorp/go-memdb.StringFieldIndex Field=Node Lowercase=true, github.com/hashicorp/go-memdb.StringFieldIndex Field=ServiceID Lowercase=true] AllowMissing=false - index=node_service_check allow-missing - indexer=github.com/hashicorp/go-memdb.CompoundIndex Indexes=[github.com/hashicorp/go-memdb.StringFieldIndex Field=Node Lowercase=true, github.com/hashicorp/go-memdb.FieldSetIndex Field=ServiceID] AllowMissing=false + indexer=github.com/hashicorp/consul/agent/consul/state.indexerSingle fromArgsIndexer=github.com/hashicorp/consul/agent/consul/state.indexFromSingleArg.func1 fromObjectIndexer=github.com/hashicorp/consul/agent/consul/state.indexNodeServiceFromHealthCheck index=service allow-missing indexer=github.com/hashicorp/go-memdb.StringFieldIndex Field=ServiceName Lowercase=true index=status