Remove StringInterner from nodedb (#3844)

* remove stringinterner from nodedb Signed-off-by: Chris Martin <[email protected]> * fix case of labels being nil Signed-off-by: Chris Martin <[email protected]> --------- Signed-off-by: Chris Martin <[email protected]> Co-authored-by: Chris Martin <[email protected]>
armadaproject · Jul 29, 2024 · 29c293e · 29c293e
1 parent 18db6e4
commit 29c293e
Show file tree

Hide file tree

Showing 9 changed files with 5 additions and 37 deletions.
diff --git a/internal/scheduler/gang_scheduler_test.go b/internal/scheduler/gang_scheduler_test.go
@@ -13,7 +13,6 @@ import (
 
 	"github.com/armadaproject/armada/internal/common/armadacontext"
 	armadaslices "github.com/armadaproject/armada/internal/common/slices"
-	"github.com/armadaproject/armada/internal/common/stringinterner"
 	"github.com/armadaproject/armada/internal/common/types"
 	"github.com/armadaproject/armada/internal/common/util"
 	"github.com/armadaproject/armada/internal/scheduler/configuration"
@@ -528,7 +527,6 @@ func TestGangScheduler(t *testing.T) {
 				tc.SchedulingConfig.IndexedTaints,
 				tc.SchedulingConfig.IndexedNodeLabels,
 				tc.SchedulingConfig.WellKnownNodeTypes,
-				stringinterner.New(1024),
 				testfixtures.TestResourceListFactory,
 			)
 			require.NoError(t, err)

diff --git a/internal/scheduler/nodedb/nodedb.go b/internal/scheduler/nodedb/nodedb.go
@@ -16,7 +16,6 @@ import (
 
 	"github.com/armadaproject/armada/internal/common/armadaerrors"
 	"github.com/armadaproject/armada/internal/common/slices"
-	"github.com/armadaproject/armada/internal/common/stringinterner"
 	"github.com/armadaproject/armada/internal/common/types"
 	"github.com/armadaproject/armada/internal/scheduler/configuration"
 	schedulercontext "github.com/armadaproject/armada/internal/scheduler/context"
@@ -39,10 +38,13 @@ var empty struct{}
 func (nodeDb *NodeDb) create(node *schedulerobjects.Node) (*internaltypes.Node, error) {
 	taints := node.GetTaints()
 	if node.Unschedulable {
-		taints = append(koTaint.DeepCopyTaintsInternStrings(taints, nodeDb.stringInterner.Intern), UnschedulableTaint())
+		taints = append(koTaint.DeepCopyTaints(taints), UnschedulableTaint())
 	}
 
-	labels := nodeDb.copyMapWithIntern(node.GetLabels())
+	labels := maps.Clone(node.GetLabels())
+	if labels == nil {
+		labels = map[string]string{}
+	}
 	labels[configuration.NodeIdLabel] = node.Id
 
 	totalResources := node.TotalResources
@@ -97,14 +99,6 @@ func (nodeDb *NodeDb) create(node *schedulerobjects.Node) (*internaltypes.Node,
 		nil), nil
 }
 
-func (nodeDb *NodeDb) copyMapWithIntern(labels map[string]string) map[string]string {
-	result := make(map[string]string, len(labels))
-	for k, v := range labels {
-		result[nodeDb.stringInterner.Intern(k)] = nodeDb.stringInterner.Intern(v)
-	}
-	return result
-}
-
 func (nodeDb *NodeDb) CreateAndInsertWithJobDbJobsWithTxn(txn *memdb.Txn, jobs []*jobdb.Job, node *schedulerobjects.Node) error {
 	entry, err := nodeDb.create(node)
 	if err != nil {
@@ -217,8 +211,6 @@ type NodeDb struct {
 	// scheduling round uses a fresh NodeDb.
 	scheduledAtPriorityByJobId map[string]int32
 
-	stringInterner *stringinterner.StringInterner
-
 	resourceListFactory *internaltypes.ResourceListFactory
 }
 
@@ -228,7 +220,6 @@ func NewNodeDb(
 	indexedTaints []string,
 	indexedNodeLabels []string,
 	wellKnownNodeTypes []configuration.WellKnownNodeType,
-	stringInterner *stringinterner.StringInterner,
 	resourceListFactory *internaltypes.ResourceListFactory,
 ) (*NodeDb, error) {
 	nodeDbPriorities := []int32{evictedPriority}
@@ -285,7 +276,6 @@ func NewNodeDb(
 		podRequirementsNotMetReasonStringCache: make(map[uint64]string, 128),
 
 		scheduledAtPriorityByJobId: make(map[string]int32),
-		stringInterner:             stringInterner,
 		resourceListFactory:        resourceListFactory,
 	}
 

diff --git a/internal/scheduler/nodedb/nodedb_test.go b/internal/scheduler/nodedb/nodedb_test.go
@@ -13,7 +13,6 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 
 	armadamaps "github.com/armadaproject/armada/internal/common/maps"
-	"github.com/armadaproject/armada/internal/common/stringinterner"
 	"github.com/armadaproject/armada/internal/common/util"
 	schedulerconfig "github.com/armadaproject/armada/internal/scheduler/configuration"
 	schedulercontext "github.com/armadaproject/armada/internal/scheduler/context"
@@ -556,7 +555,6 @@ func TestAwayNodeTypes(t *testing.T) {
 		testfixtures.TestIndexedTaints,
 		testfixtures.TestIndexedNodeLabels,
 		testfixtures.TestWellKnownNodeTypes,
-		stringinterner.New(1024),
 		testfixtures.TestResourceListFactory,
 	)
 	require.NoError(t, err)
@@ -723,7 +721,6 @@ func benchmarkUpsert(nodes []*schedulerobjects.Node, b *testing.B) {
 		testfixtures.TestIndexedTaints,
 		testfixtures.TestIndexedNodeLabels,
 		testfixtures.TestWellKnownNodeTypes,
-		stringinterner.New(1024),
 		testfixtures.TestResourceListFactory,
 	)
 	require.NoError(b, err)
@@ -763,7 +760,6 @@ func benchmarkScheduleMany(b *testing.B, nodes []*schedulerobjects.Node, jobs []
 		testfixtures.TestIndexedTaints,
 		testfixtures.TestIndexedNodeLabels,
 		testfixtures.TestWellKnownNodeTypes,
-		stringinterner.New(1024),
 		testfixtures.TestResourceListFactory,
 	)
 	require.NoError(b, err)
@@ -889,7 +885,6 @@ func newNodeDbWithNodes(nodes []*schedulerobjects.Node) (*NodeDb, error) {
 		testfixtures.TestIndexedTaints,
 		testfixtures.TestIndexedNodeLabels,
 		testfixtures.TestWellKnownNodeTypes,
-		stringinterner.New(1024),
 		testfixtures.TestResourceListFactory,
 	)
 	if err != nil {

diff --git a/internal/scheduler/queue_scheduler_test.go b/internal/scheduler/queue_scheduler_test.go
@@ -684,7 +684,6 @@ func NewNodeDb(config configuration.SchedulingConfig, stringInterner *stringinte
 		config.IndexedTaints,
 		config.IndexedNodeLabels,
 		config.WellKnownNodeTypes,
-		stringInterner,
 		testfixtures.TestResourceListFactory,
 	)
 	if err != nil {

diff --git a/internal/scheduler/schedulerapp.go b/internal/scheduler/schedulerapp.go
@@ -234,7 +234,6 @@ func Run(config schedulerconfig.Configuration) error {
 		executorRepository,
 		queueCache,
 		schedulingContextRepository,
-		stringInterner,
 		resourceListFactory,
 		floatingResourceTypes,
 	)

diff --git a/internal/scheduler/scheduling_algo.go b/internal/scheduler/scheduling_algo.go
@@ -18,7 +18,6 @@ import (
 	"github.com/armadaproject/armada/internal/common/logging"
 	armadamaps "github.com/armadaproject/armada/internal/common/maps"
 	armadaslices "github.com/armadaproject/armada/internal/common/slices"
-	"github.com/armadaproject/armada/internal/common/stringinterner"
 	"github.com/armadaproject/armada/internal/scheduler/configuration"
 	schedulerconstraints "github.com/armadaproject/armada/internal/scheduler/constraints"
 	schedulercontext "github.com/armadaproject/armada/internal/scheduler/context"
@@ -57,7 +56,6 @@ type FairSchedulingAlgo struct {
 	// Pools that need to be scheduled in sorted order
 	poolsToSchedule       []string
 	clock                 clock.Clock
-	stringInterner        *stringinterner.StringInterner
 	resourceListFactory   *internaltypes.ResourceListFactory
 	floatingResourceTypes *floatingresources.FloatingResourceTypes
 }
@@ -68,7 +66,6 @@ func NewFairSchedulingAlgo(
 	executorRepository database.ExecutorRepository,
 	queueCache queue.QueueCache,
 	schedulingContextRepository *reports.SchedulingContextRepository,
-	stringInterner *stringinterner.StringInterner,
 	resourceListFactory *internaltypes.ResourceListFactory,
 	floatingResourceTypes *floatingresources.FloatingResourceTypes,
 ) (*FairSchedulingAlgo, error) {
@@ -87,7 +84,6 @@ func NewFairSchedulingAlgo(
 		limiterByQueue:              make(map[string]*rate.Limiter),
 		maxSchedulingDuration:       maxSchedulingDuration,
 		clock:                       clock.RealClock{},
-		stringInterner:              stringInterner,
 		resourceListFactory:         resourceListFactory,
 		floatingResourceTypes:       floatingResourceTypes,
 	}, nil
@@ -410,7 +406,6 @@ func (l *FairSchedulingAlgo) schedulePool(
 		l.schedulingConfig.IndexedTaints,
 		l.schedulingConfig.IndexedNodeLabels,
 		l.schedulingConfig.WellKnownNodeTypes,
-		l.stringInterner,
 		l.resourceListFactory,
 	)
 	if err != nil {

diff --git a/internal/scheduler/scheduling_algo_test.go b/internal/scheduler/scheduling_algo_test.go
@@ -14,7 +14,6 @@ import (
 
 	"github.com/armadaproject/armada/internal/common/armadacontext"
 	armadaslices "github.com/armadaproject/armada/internal/common/slices"
-	"github.com/armadaproject/armada/internal/common/stringinterner"
 	"github.com/armadaproject/armada/internal/scheduler/configuration"
 	"github.com/armadaproject/armada/internal/scheduler/jobdb"
 	schedulermocks "github.com/armadaproject/armada/internal/scheduler/mocks"
@@ -407,7 +406,6 @@ func TestSchedule(t *testing.T) {
 				mockExecutorRepo,
 				mockQueueCache,
 				schedulingContextRepo,
-				stringinterner.New(1024),
 				testfixtures.TestResourceListFactory,
 				testfixtures.TestEmptyFloatingResources,
 			)
@@ -567,14 +565,12 @@ func BenchmarkNodeDbConstruction(b *testing.B) {
 			b.ResetTimer()
 			for n := 0; n < b.N; n++ {
 				b.StopTimer()
-				stringInterner := stringinterner.New(1024)
 				algo, err := NewFairSchedulingAlgo(
 					schedulingConfig,
 					time.Second*5,
 					nil,
 					nil,
 					nil,
-					stringInterner,
 					testfixtures.TestResourceListFactory,
 					testfixtures.TestEmptyFloatingResources,
 				)
@@ -587,7 +583,6 @@ func BenchmarkNodeDbConstruction(b *testing.B) {
 					schedulingConfig.IndexedTaints,
 					schedulingConfig.IndexedNodeLabels,
 					schedulingConfig.WellKnownNodeTypes,
-					stringInterner,
 					testfixtures.TestResourceListFactory,
 				)
 				require.NoError(b, err)

diff --git a/internal/scheduler/simulator/simulator.go b/internal/scheduler/simulator/simulator.go
@@ -262,7 +262,6 @@ func (s *Simulator) setupClusters() error {
 				s.schedulingConfig.IndexedTaints,
 				s.schedulingConfig.IndexedNodeLabels,
 				s.schedulingConfig.WellKnownNodeTypes,
-				stringinterner.New(1024),
 				s.resourceListFactory,
 			)
 			if err != nil {

diff --git a/internal/scheduler/submitcheck.go b/internal/scheduler/submitcheck.go
@@ -13,7 +13,6 @@ import (
 	"github.com/armadaproject/armada/internal/common/armadacontext"
 	"github.com/armadaproject/armada/internal/common/logging"
 	armadaslices "github.com/armadaproject/armada/internal/common/slices"
-	"github.com/armadaproject/armada/internal/common/stringinterner"
 	"github.com/armadaproject/armada/internal/scheduler/configuration"
 	schedulercontext "github.com/armadaproject/armada/internal/scheduler/context"
 	"github.com/armadaproject/armada/internal/scheduler/database"
@@ -262,7 +261,6 @@ func (srv *SubmitChecker) constructNodeDb(nodes []*schedulerobjects.Node) (*node
 		srv.schedulingConfig.IndexedTaints,
 		srv.schedulingConfig.IndexedNodeLabels,
 		srv.schedulingConfig.WellKnownNodeTypes,
-		stringinterner.New(10000),
 		srv.resourceListFactory,
 	)
 	if err != nil {