Skip to content

Commit ce6a244

Browse files
committed
Fix Goutham's comments
Signed-off-by: Ganesh Vernekar <[email protected]>
1 parent 4dd4ce6 commit ce6a244

File tree

6 files changed

+63
-108
lines changed

6 files changed

+63
-108
lines changed

pkg/ingester/label_pairs.go

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -88,25 +88,3 @@ func (a labelPairs) equal(b labels.Labels) bool {
8888
}
8989
return true
9090
}
91-
92-
func newLabelPairs(metric labels.Labels) []client.LabelPair {
93-
lp := make([]client.LabelPair, 0, len(metric))
94-
for _, m := range metric {
95-
lp = append(lp, client.LabelPair{
96-
Name: []byte(m.Name),
97-
Value: []byte(m.Value),
98-
})
99-
}
100-
return lp
101-
}
102-
103-
func newLabelPairsFromLabelAdapters(metric []client.LabelAdapter) []client.LabelPair {
104-
lp := make([]client.LabelPair, 0, len(metric))
105-
for _, m := range metric {
106-
lp = append(lp, client.LabelPair{
107-
Name: []byte(m.Name),
108-
Value: []byte(m.Value),
109-
})
110-
}
111-
return lp
112-
}

pkg/ingester/series.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010
"github.com/prometheus/prometheus/pkg/value"
1111

1212
"github.com/cortexproject/cortex/pkg/chunk/encoding"
13-
"github.com/cortexproject/cortex/pkg/ingester/client"
1413
"github.com/cortexproject/cortex/pkg/prom1/storage/metric"
1514
)
1615

@@ -51,11 +50,6 @@ func newMemorySeries(m labels.Labels) *memorySeries {
5150
}
5251
}
5352

54-
// helper to extract the not-necessarily-sorted type used elsewhere, without casting everywhere.
55-
func (s *memorySeries) labels() []client.LabelPair {
56-
return newLabelPairs(s.metric)
57-
}
58-
5953
// add adds a sample pair to the series, possibly creating a new chunk.
6054
// The caller must have locked the fingerprint of the series.
6155
func (s *memorySeries) add(v model.SamplePair) error {

pkg/ingester/user_state.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ func (u *userState) createSeriesWithFingerprint(fp model.Fingerprint, metric lab
231231
if record != nil {
232232
record.Labels = append(record.Labels, Labels{
233233
Fingerprint: uint64(fp),
234-
Labels: newLabelPairsFromLabelAdapters(metric),
234+
Labels: metric,
235235
})
236236
}
237237

pkg/ingester/wal.go

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ func (w *walWrapper) checkpointSeries(cp *wal.WAL, userID string, fp model.Finge
351351
buf, err := proto.Marshal(&Series{
352352
UserId: userID,
353353
Fingerprint: uint64(fp),
354-
Labels: series.labels(),
354+
Labels: client.FromLabelsToLabelAdapters(series.metric),
355355
Chunks: wireChunks,
356356
})
357357
if err != nil {
@@ -472,6 +472,11 @@ Loop:
472472
if err := proto.Unmarshal(reader.Record(), s); err != nil {
473473
return err
474474
}
475+
// The yoloString from the unmarshal of LabelAdapter gets corrupted
476+
// when travelling through the channel. Hence making a copy of that.
477+
// This extra alloc during the read path is fine as it's only 1 time
478+
// and saves extra allocs during write path by having LabelAdapter.
479+
s.Labels = copyLabelAdapters(s.Labels)
475480

476481
select {
477482
case errFromChan = <-errChan:
@@ -502,6 +507,17 @@ Loop:
502507
return nil
503508
}
504509

510+
func copyLabelAdapters(las []client.LabelAdapter) []client.LabelAdapter {
511+
for i := range las {
512+
n, v := make([]byte, len(las[i].Name)), make([]byte, len(las[i].Value))
513+
copy(n, las[i].Name)
514+
copy(v, las[i].Value)
515+
las[i].Name = string(n)
516+
las[i].Value = string(v)
517+
}
518+
return las
519+
}
520+
505521
func processCheckpointRecord(userStates *userStates, seriesPool *sync.Pool, stateCache map[string]*userState,
506522
seriesCache map[string]map[uint64]*memorySeries, seriesChan <-chan *Series, errChan chan error) {
507523
var la []client.LabelAdapter
@@ -580,7 +596,6 @@ func processWAL(name string, startSegment int, userStates *userStates, nWorkers
580596
defer closer.Close()
581597

582598
var (
583-
la []client.LabelAdapter
584599
errFromChan error
585600
record = &Record{}
586601
)
@@ -605,15 +620,7 @@ Loop:
605620
if ok {
606621
continue
607622
}
608-
609-
la = la[:0]
610-
for _, l := range labels.Labels {
611-
la = append(la, client.LabelAdapter{
612-
Name: string(l.Name),
613-
Value: string(l.Value),
614-
})
615-
}
616-
_, err := state.createSeriesWithFingerprint(model.Fingerprint(labels.Fingerprint), la, nil, true)
623+
_, err := state.createSeriesWithFingerprint(model.Fingerprint(labels.Fingerprint), labels.Labels, nil, true)
617624
if err != nil {
618625
return err
619626
}

pkg/ingester/wal.pb.go

Lines changed: 42 additions & 66 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/ingester/wal.proto

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ message Record {
1515

1616
message Labels {
1717
uint64 fingerprint = 1;
18-
repeated cortex.LabelPair labels = 2 [(gogoproto.nullable) = false];
18+
repeated cortex.LabelPair labels = 2 [(gogoproto.nullable) = false, (gogoproto.customtype) = "github.com/cortexproject/cortex/pkg/ingester/client.LabelAdapter"];
1919
}
2020

2121
message Sample {
@@ -27,6 +27,6 @@ message Sample {
2727
message Series {
2828
string user_id = 1;
2929
uint64 fingerprint = 2;
30-
repeated cortex.LabelPair labels = 3 [(gogoproto.nullable) = false];
30+
repeated cortex.LabelPair labels = 3 [(gogoproto.nullable) = false, (gogoproto.customtype) = "github.com/cortexproject/cortex/pkg/ingester/client.LabelAdapter"];
3131
repeated cortex.Chunk chunks = 4 [(gogoproto.nullable) = false];
3232
}

0 commit comments

Comments
 (0)