Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backupccl: add BackupMonitor to memory monitor file stitching #73805

Merged
merged 1 commit into from
Dec 20, 2021

Conversation

adityamaru
Copy link
Contributor

@adityamaru adityamaru commented Dec 14, 2021

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 dt and a team and removed request for a team December 14, 2021 18:49
@cockroach-teamcity
Copy link
Member

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
Copy link
Member

@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

pkg/ccl/backupccl/backup_processor.go Outdated Show resolved Hide resolved
@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
Contributor Author

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.

@adityamaru
Copy link
Contributor Author

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

TFTR!
bors r=dt

@craig
Copy link
Contributor

craig bot commented Dec 18, 2021

Build failed:

@adityamaru
Copy link
Contributor Author

TestBenchmarkExpectation/DropDatabase/drop_database_0_tables looks like a flake.

bors r=dt

@craig
Copy link
Contributor

craig bot commented Dec 18, 2021

Build failed:

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
Contributor Author

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

bors r=dt

@craig
Copy link
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

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.

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