Skip to content

Commit

Permalink
perf: Remove expensive reflection from raft/mesh hot path
Browse files Browse the repository at this point in the history
Replaces a reflection-based copy of a struct in the mesh topology with a
deep-copy generated implementation.

This is in the hot-path of raft FSM updates, and the reflection overhead was a
substantial part of mesh registration times (~90%). This could manifest as raft
thread saturation, and resulting instability.

Co-authored-by: Joel Brandhorst <joel.brandhorst@gmail.com>
  • Loading branch information
2 people authored and jmurret committed May 26, 2023
1 parent 7177aad commit 04b6a90
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 17 deletions.
1 change: 1 addition & 0 deletions GNUmakefile
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ codegen-tools:
deep-copy: codegen-tools
@$(SHELL) $(CURDIR)/agent/structs/deep-copy.sh
@$(SHELL) $(CURDIR)/agent/proxycfg/deep-copy.sh
@$(SHELL) $(CURDIR)/agent/consul/state/deep-copy.sh

version:
@echo -n "Version: "
Expand Down
19 changes: 2 additions & 17 deletions agent/consul/state/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"strings"

"github.com/hashicorp/go-memdb"
"github.com/mitchellh/copystructure"

"github.com/hashicorp/consul/acl"
"github.com/hashicorp/consul/agent/configentry"
Expand Down Expand Up @@ -4711,14 +4710,7 @@ func updateMeshTopology(tx WriteTxn, idx uint64, node string, svc *structs.NodeS

var mapping *upstreamDownstream
if existing, ok := obj.(*upstreamDownstream); ok {
rawCopy, err := copystructure.Copy(existing)
if err != nil {
return fmt.Errorf("failed to copy existing topology mapping: %v", err)
}
mapping, ok = rawCopy.(*upstreamDownstream)
if !ok {
return fmt.Errorf("unexpected topology type %T", rawCopy)
}
mapping := existing.DeepCopy()
mapping.Refs[uid] = struct{}{}
mapping.ModifyIndex = idx

Expand Down Expand Up @@ -4784,14 +4776,7 @@ func cleanupMeshTopology(tx WriteTxn, idx uint64, service *structs.ServiceNode)

// Do the updates in a separate loop so we don't trash the iterator.
for _, m := range mappings {
rawCopy, err := copystructure.Copy(m)
if err != nil {
return fmt.Errorf("failed to copy existing topology mapping: %v", err)
}
copy, ok := rawCopy.(*upstreamDownstream)
if !ok {
return fmt.Errorf("unexpected topology type %T", rawCopy)
}
copy := m.DeepCopy()

// Bail early if there's no reference to the proxy ID we're deleting
if _, ok := copy.Refs[uid]; !ok {
Expand Down
15 changes: 15 additions & 0 deletions agent/consul/state/catalog_schema.deepcopy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// generated by deep-copy -pointer-receiver -o ./catalog_schema.deepcopy.go -type upstreamDownstream ./; DO NOT EDIT.

package state

// DeepCopy generates a deep copy of *upstreamDownstream
func (o *upstreamDownstream) DeepCopy() *upstreamDownstream {
var cp upstreamDownstream = *o
if o.Refs != nil {
cp.Refs = make(map[string]struct{}, len(o.Refs))
for k2, v2 := range o.Refs {
cp.Refs[k2] = v2
}
}
return &cp
}
11 changes: 11 additions & 0 deletions agent/consul/state/deep-copy.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/usr/bin/env bash

readonly PACKAGE_DIR="$(dirname "${BASH_SOURCE[0]}")"
cd $PACKAGE_DIR

# Uses: https://github.com/globusdigital/deep-copy
deep-copy \
-pointer-receiver \
-o ./catalog_schema.deepcopy.go \
-type upstreamDownstream \
./

0 comments on commit 04b6a90

Please sign in to comment.