Skip to content

limit,storage: add more trace spans to backup path#67524

Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom
adityamaru:add-export-concurrency-trace-info
Jul 14, 2021
Merged

limit,storage: add more trace spans to backup path#67524
craig[bot] merged 2 commits intocockroachdb:masterfrom
adityamaru:add-export-concurrency-trace-info

Conversation

@adityamaru
Copy link
Copy Markdown
Contributor

This change adds a trace recording to track how
many requests are waiting in the the concurrent limiter
queue.

The change also adds a child span to ExportMVCCToSst
to track how long the scan+SST creation is taking per
ExportRequest during backup.

Release note: None

@adityamaru adityamaru requested review from a team and dt July 13, 2021 03:45
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@adityamaru adityamaru force-pushed the add-export-concurrency-trace-info branch from 0bb36ae to 7cd7c3d Compare July 13, 2021 14:07
const big = 1 << 30
sstFile := &MemFile{}
_, _, err := e.ExportMVCCToSst(startKey, endKey, startTime, endTime, revisions, big, big,
_, _, err := e.ExportMVCCToSst(ctx, startKey, endKey, startTime, endTime, revisions, big, big,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2¢: I think you could just context.Background here instead to eliminate a lot of the test diff churn 🤷

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

var span *tracing.Span
ctx, span = tracing.ChildSpan(ctx, l.spanName)
defer span.Finish()
span.RecordStructured(&types.StringValue{Value: fmt.Sprintf("%d requests are waiting in the pool", l.sem.Len())})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: s/in the pool// (also, I could see this file being separate commit)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

@adityamaru adityamaru force-pushed the add-export-concurrency-trace-info branch 3 times, most recently from 77abf73 to 3f09399 Compare July 13, 2021 16:47
@adityamaru adityamaru force-pushed the add-export-concurrency-trace-info branch from 3f09399 to fe57f71 Compare July 13, 2021 17:42
@adityamaru
Copy link
Copy Markdown
Contributor Author

TFTR!

bors r=dt

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jul 14, 2021

Build failed (retrying...):

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jul 14, 2021

Build succeeded:

@craig craig bot merged commit 5a0b11e into cockroachdb:master Jul 14, 2021
@adityamaru adityamaru deleted the add-export-concurrency-trace-info branch July 14, 2021 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants