Skip to content

backupccl: add BackupMonitor to memory monitor file stitching#73805

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
adityamaru:add-mem-monitor
Dec 20, 2021
Merged

backupccl: add BackupMonitor to memory monitor file stitching#73805
craig[bot] merged 1 commit intocockroachdb:masterfrom
adityamaru:add-mem-monitor

Conversation

@adityamaru
Copy link
Copy Markdown
Contributor

@adityamaru adityamaru commented Dec 14, 2021 •
edited
Loading

This change adds a BackupMonitor that hangs off the bulk memory
monitor. This monitor currently only guards the queue that we use
to buffer SSTs while stitching them together in the sstSink.

Informs: #73815

Release note: None

@adityamaru adityamaru requested review from a team and dt and removed request for a team December 14, 2021 18:49
@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Dec 14, 2021

This change is Reviewable

@adityamaru adityamaru force-pushed the add-mem-monitor branch 5 times, most recently from 92b9b78 to 65c5ab6 Compare December 15, 2021 14:38
dt
dt reviewed Dec 16, 2021
View reviewed changes
dt
dt approved these changes Dec 17, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@dt dt left a comment

Choose a reason for hiding this comment

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

I don't immediately see why we need all the nil check no-ops on the accumulator but otherwise looks great

dt
dt reviewed Dec 17, 2021
View reviewed changes
@adityamaru adityamaru requested a review from dt December 17, 2021 03:00
@adityamaru adityamaru force-pushed the add-mem-monitor branch 2 times, most recently from 4982a6d to 2ef6458 Compare December 17, 2021 19:59
@adityamaru
Copy link
Copy Markdown
Contributor Author

adityamaru commented Dec 17, 2021

I do not understand the Bazel timeouts in testrace and stress:

[Run stress tests] _cgo_callers: relocation target x_cgo_callers not defined
[11:37:21][Run stress tests] _cgo_init: relocation target x_cgo_init not defined
[11:37:21][Run stress tests] _cgo_mmap: relocation target x_cgo_mmap not defined
[11:37:21][Run stress tests] _cgo_munmap: relocation target x_cgo_munmap not defined
[11:37:21][Run stress tests] _cgo_notify_runtime_init_done: relocation target x_cgo_notify_runtime_init_done not defined
[11:37:21][Run stress tests] _cgo_sigaction: relocation target x_cgo_sigaction not defined
[11:37:21][Run stress tests] _cgo_thread_start: relocation target x_cgo_thread_start not defined
[11:37:21][Run stress tests] external/go_sdk/pkg/tool/linux_amd64/link: too many errors
[11:37:21][Run stress tests] unexpected fault address 0x7f219d6c8000
[11:37:21][Run stress tests] fatal error: fault
[11:37:21][Run stress tests] unexpected fault address 0x7f219d61b000
[11:37:21][Run stress tests] fatal error: fault
[11:37:21][Run stress tests] link: error running subcommand external/go_sdk/pkg/tool/linux_amd64/link: exit status 2
[11:37:21][Run stress tests] Target //pkg/ccl/backupccl:backupccl_test failed to build
[11:37:21][Run stress tests] Use --verbose_failures to see the command lines of failed build steps.
[11:37:21][Run stress tests] INFO: Elapsed time: 75.045s, Critical Path: 46.72s
[11:37:21][Run stress tests] INFO: 795 processes: 317 internal, 478 processwrapper-sandbox.
[11:37:21][Run stress tests] FAILED: Build did NOT complete successfully
[11:37:21][Run stress tests] FAILED: Build did NOT complete successfully
[11:37:21][Run stress tests] ERROR: exit status 1

Rebased to try and kickoff another run to see if that fixes it.

@rickystewart rickystewart mentioned this pull request Dec 17, 2021
Closed
@adityamaru
Copy link
Copy Markdown
Contributor Author

adityamaru commented Dec 18, 2021

Bazel team is aware of the linker error. Going to go ahead and bors this.

TFTR!
bors r=dt

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 18, 2021

Build failed:

@adityamaru
Copy link
Copy Markdown
Contributor Author

adityamaru commented Dec 18, 2021

TestBenchmarkExpectation/DropDatabase/drop_database_0_tables looks like a flake.

bors r=dt

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 18, 2021

Build failed:

@adityamaru
backupccl: add BackupMonitor to memory monitor file stitching
1ac3e93 
This change adds a BackupMonitor that hangs off the bulk memory
monitor. This monitor currently only guards the queue that we use
to buffer SSTs while stitching them together in the sstSink.

Release note: None
@adityamaru
Copy link
Copy Markdown
Contributor Author

adityamaru commented Dec 20, 2021

Lets try this again, bazel failure is the known linker error.

bors r=dt

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 20, 2021

Build succeeded:

@craig craig bot merged commit 0d07a2d into cockroachdb:master Dec 20, 2021
@blathers-crl
Copy link
Copy Markdown

blathers-crl bot commented Dec 20, 2021

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.

error creating merge commit from 1ac3e93 to blathers/backport-release-21.2-73805: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict []

you may need to manually resolve merge conflicts with the backport tool.

Backport to branch 21.2.x failed. See errors above.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.

@adityamaru adityamaru mentioned this pull request Dec 21, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dt dt Awaiting requested review from dt

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@adityamaru @cockroach-teamcity @dt