Fix a couple of bugs from manual testing:

- DynamoDB won't accept empty keys; they should just be omitted. - Deal with case where two schemas start at the same time.
cortexproject · Feb 6, 2017 · ee0abb5 · ee0abb5
1 parent 2aa4ce2
commit ee0abb5
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 7 deletions.
diff --git a/chunk/chunk_store.go b/chunk/chunk_store.go
@@ -373,15 +373,17 @@ func (c *AWSStore) lookupEntry(ctx context.Context, entry IndexEntry, matcher *m
 				},
 				ComparisonOperator: aws.String("EQ"),
 			},
-			rangeKey: {
-				AttributeValueList: []*dynamodb.AttributeValue{
-					{B: entry.RangeKey},
-				},
-				ComparisonOperator: aws.String(dynamodb.ComparisonOperatorBeginsWith),
-			},
 		},
 		ReturnConsumedCapacity: aws.String(dynamodb.ReturnConsumedCapacityTotal),
 	}
+	if len(entry.RangeKey) > 0 {
+		input.KeyConditions[rangeKey] = &dynamodb.Condition{
+			AttributeValueList: []*dynamodb.AttributeValue{
+				{B: entry.RangeKey},
+			},
+			ComparisonOperator: aws.String(dynamodb.ComparisonOperatorBeginsWith),
+		}
+	}
 
 	var chunkSet ByID
 	var processingError error

diff --git a/chunk/schema.go b/chunk/schema.go
@@ -148,8 +148,14 @@ func (c compositeSchema) forSchemas(from, through model.Time, callback func(from
 		if i+1 < len(c.schemas) {
 			nextSchemaStarts = c.schemas[i+1].start
 		}
-		end := min(through, nextSchemaStarts-1)
 
+		// If the next schema starts at the same time as this one,
+		// skip this one.
+		if nextSchemaStarts == c.schemas[i].start {
+			continue
+		}
+
+		end := min(through, nextSchemaStarts-1)
 		entries, err := callback(start, end, c.schemas[i].Schema)
 		if err != nil {
 			return nil, err

diff --git a/chunk/schema_test.go b/chunk/schema_test.go
@@ -107,6 +107,22 @@ func TestSchemaComposite(t *testing.T) {
 			},
 		},
 
+		// Test we get only one result when two schema start at same time
+		{
+			compositeSchema{
+				schemas: []compositeSchemaEntry{
+					{model.TimeFromUnix(0), mockSchema(1)},
+					{model.TimeFromUnix(10), mockSchema(2)},
+					{model.TimeFromUnix(10), mockSchema(3)},
+				},
+			},
+			0, 165,
+			[]result{
+				{model.TimeFromUnix(0), model.TimeFromUnix(10) - 1, mockSchema(1)},
+				{model.TimeFromUnix(10), model.TimeFromUnix(165), mockSchema(3)},
+			},
+		},
+
 		// Test all the various combination we can get when there are three schemas
 		{
 			cs, 34, 65,