Skip to content

Commit fa5a923

Browse files
craig[bot]RaduBerinde
andcommitted
Merge #144792
144792: changefeedccl: use newer generic btree r=RaduBerinde a=RaduBerinde Fixes: #144504 Release note: None Co-authored-by: Radu Berinde <[email protected]>
2 parents 6e971e8 + 205c064 commit fa5a923

File tree

6 files changed

+26
-61
lines changed

6 files changed

+26
-61
lines changed

DEPS.bzl

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7992,16 +7992,6 @@ def go_deps():
79927992
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/RaduBerinde/axisds/com_github_raduberinde_axisds-v0.0.0-20250419182453-5135a0650657.zip",
79937993
],
79947994
)
7995-
go_repository(
7996-
name = "com_github_raduberinde_btree",
7997-
build_file_proto_mode = "disable_global",
7998-
importpath = "github.com/RaduBerinde/btree",
7999-
sha256 = "1c845dc6e77f27888fa66c8521df4d1d30f3ccf6892e8c1ba0aab4db24e717c1",
8000-
strip_prefix = "github.com/RaduBerinde/[email protected]",
8001-
urls = [
8002-
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/RaduBerinde/btree/com_github_raduberinde_btree-v1.0.2-0.20250415192849-8beea01764ce.zip",
8003-
],
8004-
)
80057995
go_repository(
80067996
name = "com_github_raduberinde_btreemap",
80077997
build_file_proto_mode = "disable_global",

build/bazelutil/distdir_files.bzl

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,6 @@ DISTDIR_FILES = {
208208
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/PuerkitoBio/purell/com_github_puerkitobio_purell-v1.1.1.zip": "59e636760d7f2ab41c2f80c1784b1c73d381d44888d1999228dedd634ddcf5ed",
209209
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/PuerkitoBio/urlesc/com_github_puerkitobio_urlesc-v0.0.0-20170810143723-de5bf2ad4578.zip": "1793124273dd94e7089e95716d40529bcf70b9e87162d60218f68dde4d6aeb9d",
210210
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/RaduBerinde/axisds/com_github_raduberinde_axisds-v0.0.0-20250419182453-5135a0650657.zip": "e943d3ac26630cf9e337483450ad9db7f43b6a0372a79d5a691aad8c3c92f9e3",
211-
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/RaduBerinde/btree/com_github_raduberinde_btree-v1.0.2-0.20250415192849-8beea01764ce.zip": "1c845dc6e77f27888fa66c8521df4d1d30f3ccf6892e8c1ba0aab4db24e717c1",
212211
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/RaduBerinde/btreemap/com_github_raduberinde_btreemap-v0.0.0-20250419174037-3d62b7205d54.zip": "81b6e0391c0bceadab0bc56184a7ed25dfe81daaa04916900f66b55f34590f6b",
213212
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/SAP/go-hdb/com_github_sap_go_hdb-v0.14.1.zip": "273de28a254c39e9f24293b864c1d664488e4a5d44d535755a5e5b68ae7eed8d",
214213
"https://storage.googleapis.com/cockroach-godeps/gomod/github.com/Shopify/goreferrer/com_github_shopify_goreferrer-v0.0.0-20220729165902-8cddb4f5de06.zip": "280a2f55812e8b475cfd9d467a3b3d5859315788e68592a8fc5d6cedadc0503f",

go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ require (
104104
github.com/MichaelTJones/walk v0.0.0-20161122175330-4748e29d5718
105105
github.com/NYTimes/gziphandler v0.0.0-20170623195520-56545f4a5d46
106106
github.com/PuerkitoBio/goquery v1.5.1
107-
github.com/RaduBerinde/btree v1.0.2-0.20250415192849-8beea01764ce
108107
github.com/RaduBerinde/btreemap v0.0.0-20250419174037-3d62b7205d54
109108
github.com/VividCortex/ewma v1.1.1
110109
github.com/alessio/shellescape v1.4.1

go.sum

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,6 @@ github.com/PuerkitoBio/urlesc v0.0.0-20160726150825-5bd2802263f2/go.mod h1:uGdko
268268
github.com/PuerkitoBio/urlesc v0.0.0-20170810143723-de5bf2ad4578/go.mod h1:uGdkoq3SwY9Y+13GIhn11/XLaGBb4BfwItxLd5jeuXE=
269269
github.com/RaduBerinde/axisds v0.0.0-20250419182453-5135a0650657 h1:8XBWWQD+vFF+JqOsm16t0Kab1a7YWV8+GISVEP8AuZ8=
270270
github.com/RaduBerinde/axisds v0.0.0-20250419182453-5135a0650657/go.mod h1:UHGJonU9z4YYGKJxSaC6/TNcLOBptpmM5m2Cksbnw0Y=
271-
github.com/RaduBerinde/btree v1.0.2-0.20250415192849-8beea01764ce h1:jmg1o6EkSTLFQ4vdTBzob5AjFBzNH4ELn56iKhMVk48=
272-
github.com/RaduBerinde/btree v1.0.2-0.20250415192849-8beea01764ce/go.mod h1:1+GykKoq3bIUWDVjNWWxWYblBduYahx5NGepZjgYqvI=
273271
github.com/RaduBerinde/btreemap v0.0.0-20250419174037-3d62b7205d54 h1:bsU8Tzxr/PNz75ayvCnxKZWEYdLMPDkUgticP4a4Bvk=
274272
github.com/RaduBerinde/btreemap v0.0.0-20250419174037-3d62b7205d54/go.mod h1:0tr7FllbE9gJkHq7CVeeDDFAFKQVy5RnCSSNBOvdqbc=
275273
github.com/SAP/go-hdb v0.14.1/go.mod h1:7fdQLVC2lER3urZLjZCm0AuMQfApof92n3aylBPEkMo=

pkg/ccl/changefeedccl/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ go_library(
170170
"@com_github_klauspost_pgzip//:pgzip",
171171
"@com_github_lib_pq//:pq",
172172
"@com_github_linkedin_goavro_v2//:goavro",
173-
"@com_github_raduberinde_btree//:btree",
173+
"@com_github_raduberinde_btreemap//:btreemap",
174174
"@com_github_rcrowley_go_metrics//:go-metrics",
175175
"@com_github_twmb_franz_go//pkg/kerr",
176176
"@com_github_twmb_franz_go//pkg/kgo",

pkg/ccl/changefeedccl/sink_cloudstorage.go

Lines changed: 25 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package changefeedccl
77

88
import (
99
"bytes"
10+
"cmp"
1011
"context"
1112
"crypto/rand"
1213
"encoding/hex"
@@ -18,7 +19,7 @@ import (
1819
"sync/atomic"
1920
"time"
2021

21-
"github.com/RaduBerinde/btree" // TODO(#144504): switch to the newer btree
22+
"github.com/RaduBerinde/btreemap"
2223
"github.com/cockroachdb/cockroach/pkg/base"
2324
"github.com/cockroachdb/cockroach/pkg/ccl/changefeedccl/changefeedbase"
2425
"github.com/cockroachdb/cockroach/pkg/ccl/changefeedccl/kvevent"
@@ -320,7 +321,7 @@ type cloudStorageSink struct {
320321
// These are fields to track information needed to output files based on the naming
321322
// convention described above. See comment on cloudStorageSink above for more details.
322323
fileID int64
323-
files *btree.BTree // of *cloudStorageSinkFile
324+
files *btreemap.BTreeMap[cloudStorageSinkKey, *cloudStorageSinkFile]
324325

325326
timestampOracle timestampLowerBoundOracle
326327
jobSessionID string
@@ -416,7 +417,7 @@ func makeCloudStorageSink(
416417
sinkID: sinkID,
417418
settings: settings,
418419
targetMaxFileSize: targetMaxFileSize,
419-
files: btree.New(8),
420+
files: btreemap.New[cloudStorageSinkKey, *cloudStorageSinkFile](8, keyCmp),
420421
partitionFormat: defaultPartitionFormat,
421422
timestampOracle: timestampOracle,
422423
// TODO(dan,ajwerner): Use the jobs framework's session ID once that's available.
@@ -516,8 +517,7 @@ func (s *cloudStorageSink) getOrCreateFile(
516517
) (*cloudStorageSinkFile, error) {
517518
name, _ := s.topicNamer.Name(topic)
518519
key := cloudStorageSinkKey{name, int64(topic.GetVersion())}
519-
if item := s.files.Get(key); item != nil {
520-
f := item.(*cloudStorageSinkFile)
520+
if _, f, _ := s.files.Get(key); f != nil {
521521
if eventMVCC.Less(f.oldestMVCC) {
522522
f.oldestMVCC = eventMVCC
523523
}
@@ -537,7 +537,7 @@ func (s *cloudStorageSink) getOrCreateFile(
537537
}
538538
f.codec = codec
539539
}
540-
s.files.ReplaceOrInsert(f)
540+
s.files.ReplaceOrInsert(f.cloudStorageSinkKey, f)
541541
return f, nil
542542
}
543543

@@ -646,20 +646,17 @@ func (s *cloudStorageSink) EmitResolvedTimestamp(
646646
// on cloudStorageSink)
647647
func (s *cloudStorageSink) flushTopicVersions(
648648
ctx context.Context, topic string, maxVersionToFlush int64,
649-
) (err error) {
649+
) error {
650650
var toRemoveAlloc [2]int64 // generally avoid allocating
651651
toRemove := toRemoveAlloc[:0] // schemaIDs of flushed files
652652
gte := cloudStorageSinkKey{topic: topic}
653653
lt := cloudStorageSinkKey{topic: topic, schemaID: maxVersionToFlush + 1}
654-
s.files.AscendRange(gte, lt, func(i btree.Item) (wantMore bool) {
655-
f := i.(*cloudStorageSinkFile)
656-
if err = s.flushFile(ctx, f); err == nil {
657-
toRemove = append(toRemove, f.schemaID)
654+
655+
for _, f := range s.files.Ascend(btreemap.GE(gte), btreemap.LT(lt)) {
656+
if err := s.flushFile(ctx, f); err != nil {
657+
return err
658658
}
659-
return err == nil
660-
})
661-
if err != nil {
662-
return err
659+
toRemove = append(toRemove, f.schemaID)
663660
}
664661

665662
// Allow synchronization with the async flusher to happen.
@@ -672,8 +669,7 @@ func (s *cloudStorageSink) flushTopicVersions(
672669
// flushed files may not be removed from s.files. This is ok, since
673670
// the error will trigger the sink to be closed, and we will only use
674671
// s.files to ensure that the codecs are closed before deallocating it.
675-
err = s.waitAsyncFlush(ctx)
676-
if err != nil {
672+
if err := s.waitAsyncFlush(ctx); err != nil {
677673
return err
678674
}
679675

@@ -682,7 +678,7 @@ func (s *cloudStorageSink) flushTopicVersions(
682678
for _, v := range toRemove {
683679
s.files.Delete(cloudStorageSinkKey{topic: topic, schemaID: v})
684680
}
685-
return err
681+
return nil
686682
}
687683

688684
// Flush implements the Sink interface.
@@ -693,13 +689,10 @@ func (s *cloudStorageSink) Flush(ctx context.Context) error {
693689

694690
s.metrics.recordFlushRequestCallback()()
695691

696-
var err error
697-
s.files.Ascend(func(i btree.Item) (wantMore bool) {
698-
err = s.flushFile(ctx, i.(*cloudStorageSinkFile))
699-
return err == nil
700-
})
701-
if err != nil {
702-
return err
692+
for _, f := range s.files.Ascend(btreemap.Min[cloudStorageSinkKey](), btreemap.Max[cloudStorageSinkKey]()) {
693+
if err := s.flushFile(ctx, f); err != nil {
694+
return err
695+
}
703696
}
704697
// Allow synchronization with the async flusher to happen.
705698
if s.testingKnobs != nil && s.testingKnobs.AsyncFlushSync != nil {
@@ -711,8 +704,7 @@ func (s *cloudStorageSink) Flush(ctx context.Context) error {
711704
// flushed files may not be removed from s.files. This is ok, since
712705
// the error will trigger the sink to be closed, and we will only use
713706
// s.files to ensure that the codecs are closed before deallocating it.
714-
err = s.waitAsyncFlush(ctx)
715-
if err != nil {
707+
if err := s.waitAsyncFlush(ctx); err != nil {
716708
return err
717709
}
718710
// Files need to be cleared after the flush completes, otherwise file resources
@@ -909,17 +901,15 @@ func (s *cloudStorageSink) closeAllCodecs() (err error) {
909901
// Codecs need to be closed because of the klauspost compression library implementation
910902
// details where it spins up go routines to perform compression in parallel.
911903
// Those go routines are cleaned up when the compression codec is closed.
912-
s.files.Ascend(func(i btree.Item) (wantMore bool) {
913-
f := i.(*cloudStorageSinkFile)
904+
for _, f := range s.files.Ascend(btreemap.Min[cloudStorageSinkKey](), btreemap.Max[cloudStorageSinkKey]()) {
914905
if f.codec != nil {
915906
cErr := f.codec.Close()
916907
f.codec = nil
917908
if err == nil {
918909
err = cErr
919910
}
920911
}
921-
return true
922-
})
912+
}
923913
return err
924914
}
925915

@@ -943,22 +933,11 @@ type cloudStorageSinkKey struct {
943933
schemaID int64
944934
}
945935

946-
func (k cloudStorageSinkKey) Less(other btree.Item) bool {
947-
switch other := other.(type) {
948-
case *cloudStorageSinkFile:
949-
return keyLess(k, other.cloudStorageSinkKey)
950-
case cloudStorageSinkKey:
951-
return keyLess(k, other)
952-
default:
953-
panic(errors.Errorf("unexpected item type %T", other))
954-
}
955-
}
956-
957-
func keyLess(a, b cloudStorageSinkKey) bool {
958-
if a.topic == b.topic {
959-
return a.schemaID < b.schemaID
936+
func keyCmp(a, b cloudStorageSinkKey) int {
937+
if a.topic != b.topic {
938+
return cmp.Compare(a.topic, b.topic)
960939
}
961-
return a.topic < b.topic
940+
return cmp.Compare(a.schemaID, b.schemaID)
962941
}
963942

964943
// generateChangefeedSessionID generates a unique string that is used to

0 commit comments

Comments
 (0)