fix/feat: use joins instead of n+1/opening multiple connections (#1297)

* chore: wip sqlite concurrency issue * chore: use left join to get flags/variants at same time * chore: comments to clarify logic * chore: finish joins * chore: set maxopencons to 1 for sqlite * chore: only disable CSP headers in development * Update internal/storage/sql/db.go Co-authored-by: George <me@georgemac.com> * chore: log warning if overriding max_open_conn --------- Co-authored-by: George <me@georgemac.com>
flipt-io · Feb 1, 2023 · 6ded9f8 · 6ded9f8
1 parent ef32b3e
commit 6ded9f8
Show file tree

Hide file tree

Showing 8 changed files with 410 additions and 223 deletions.
diff --git a/internal/cmd/grpc.go b/internal/cmd/grpc.go
@@ -106,6 +106,10 @@ func NewGRPCServer(
 		return nil, fmt.Errorf("opening db: %w", err)
 	}
 
+	if driver == sql.SQLite && cfg.Database.MaxOpenConn > 1 {
+		logger.Warn("ignoring config.db.max_open_conn due to driver limitation (sqlite)", zap.Int("attempted_max_conn", cfg.Database.MaxOpenConn))
+	}
+
 	server.onShutdown(func(context.Context) error {
 		return db.Close()
 	})

diff --git a/internal/info/flipt.go b/internal/info/flipt.go
@@ -16,7 +16,7 @@ type Flipt struct {
 }
 
 func (f Flipt) IsDevelopment() bool {
-	return f.Version == "dev" && !f.IsRelease
+	return f.Version == "dev"
 }
 
 func (f Flipt) ServeHTTP(w http.ResponseWriter, r *http.Request) {

diff --git a/internal/storage/sql/common/evaluation.go b/internal/storage/sql/common/evaluation.go
@@ -9,14 +9,6 @@ import (
 	flipt "go.flipt.io/flipt/rpc/flipt"
 )
 
-type optionalConstraint struct {
-	ID       sql.NullString
-	Type     sql.NullInt64
-	Property sql.NullString
-	Operator sql.NullString
-	Value    sql.NullString
-}
-
 func (s *Store) GetEvaluationRules(ctx context.Context, flagKey string) ([]*storage.EvaluationRule, error) {
 	// get all rules for flag with their constraints if any
 	rows, err := s.builder.Select("r.id, r.flag_key, r.segment_key, s.match_type, r.\"rank\", c.id, c.type, c.property, c.operator, c.value").
@@ -38,8 +30,8 @@ func (s *Store) GetEvaluationRules(ctx context.Context, flagKey string) ([]*stor
 	}()
 
 	var (
-		seenRules = make(map[string]*storage.EvaluationRule)
-		rules     = []*storage.EvaluationRule{}
+		uniqueRules = make(map[string]*storage.EvaluationRule)
+		rules       = []*storage.EvaluationRule{}
 	)
 
 	for rows.Next() {
@@ -48,16 +40,26 @@ func (s *Store) GetEvaluationRules(ctx context.Context, flagKey string) ([]*stor
 			optionalConstraint optionalConstraint
 		)
 
-		if err := rows.Scan(&tempRule.ID, &tempRule.FlagKey, &tempRule.SegmentKey, &tempRule.SegmentMatchType, &tempRule.Rank, &optionalConstraint.ID, &optionalConstraint.Type, &optionalConstraint.Property, &optionalConstraint.Operator, &optionalConstraint.Value); err != nil {
-			return nil, err
+		if err := rows.Scan(
+			&tempRule.ID,
+			&tempRule.FlagKey,
+			&tempRule.SegmentKey,
+			&tempRule.SegmentMatchType,
+			&tempRule.Rank,
+			&optionalConstraint.Id,
+			&optionalConstraint.Type,
+			&optionalConstraint.Property,
+			&optionalConstraint.Operator,
+			&optionalConstraint.Value); err != nil {
+			return rules, err
 		}
 
-		if existingRule, ok := seenRules[tempRule.ID]; ok {
+		if existingRule, ok := uniqueRules[tempRule.ID]; ok {
 			// current rule we know about
-			if optionalConstraint.ID.Valid {
+			if optionalConstraint.Id.Valid {
 				constraint := storage.EvaluationConstraint{
-					ID:       optionalConstraint.ID.String,
-					Type:     flipt.ComparisonType(optionalConstraint.Type.Int64),
+					ID:       optionalConstraint.Id.String,
+					Type:     flipt.ComparisonType(optionalConstraint.Type.Int32),
 					Property: optionalConstraint.Property.String,
 					Operator: optionalConstraint.Operator.String,
 					Value:    optionalConstraint.Value.String,
@@ -74,24 +76,28 @@ func (s *Store) GetEvaluationRules(ctx context.Context, flagKey string) ([]*stor
 				Rank:             tempRule.Rank,
 			}
 
-			if optionalConstraint.ID.Valid {
+			if optionalConstraint.Id.Valid {
 				constraint := storage.EvaluationConstraint{
-					ID:       optionalConstraint.ID.String,
-					Type:     flipt.ComparisonType(optionalConstraint.Type.Int64),
+					ID:       optionalConstraint.Id.String,
+					Type:     flipt.ComparisonType(optionalConstraint.Type.Int32),
 					Property: optionalConstraint.Property.String,
 					Operator: optionalConstraint.Operator.String,
 					Value:    optionalConstraint.Value.String,
 				}
 				newRule.Constraints = append(newRule.Constraints, constraint)
 			}
 
-			seenRules[newRule.ID] = newRule
+			uniqueRules[newRule.ID] = newRule
 			rules = append(rules, newRule)
 		}
 	}
 
 	if err := rows.Err(); err != nil {
-		return nil, err
+		return rules, err
+	}
+
+	if err := rows.Close(); err != nil {
+		return rules, err
 	}
 
 	return rules, nil
@@ -125,13 +131,13 @@ func (s *Store) GetEvaluationDistributions(ctx context.Context, ruleID string) (
 		if err := rows.Scan(
 			&d.ID, &d.RuleID, &d.VariantID, &d.Rollout, &d.VariantKey, &attachment,
 		); err != nil {
-			return nil, err
+			return distributions, err
 		}
 
 		if attachment.Valid {
 			attachmentString, err := compactJSONString(attachment.String)
 			if err != nil {
-				return nil, err
+				return distributions, err
 			}
 			d.VariantAttachment = attachmentString
 		}
@@ -140,7 +146,11 @@ func (s *Store) GetEvaluationDistributions(ctx context.Context, ruleID string) (
 	}
 
 	if err := rows.Err(); err != nil {
-		return nil, err
+		return distributions, err
+	}
+
+	if err := rows.Close(); err != nil {
+		return distributions, err
 	}
 
 	return distributions, nil