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

[tests] Improved embedded coordinator support #3779

Merged
merged 3 commits into from
Sep 28, 2021
Merged

Conversation

nbroyles
Copy link
Collaborator

What this PR does / why we need it:

This commit fixes a couple small things re: embedded coordinators:

  1. Fails fast if dbnode is not started when creating an embedded
    coordinator.

  2. Adds support for port and filepath replacement in embedded
    config.

Special notes for your reviewer:

Does this PR introduce a user-facing and/or backwards incompatible change?:


Does this PR require updating code package or user-facing documentation?:


@nbroyles
Copy link
Collaborator Author

@jaredjenkins PR addresses this comment: #3759 (comment)

@nbroyles nbroyles force-pushed the nb/dbnode-started branch 2 times, most recently from b9d3f00 to 7632e3a Compare September 28, 2021 15:07
@codecov
Copy link

codecov bot commented Sep 28, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@9778df4). Click here to learn what that means.
The diff coverage is 15.7%.

❗ Current head 7632e3a differs from pull request most recent head 521e70e. Consider uploading reports for the commit 521e70e to get more accurate results

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #3779   +/-   ##
========================================
  Coverage          ?   56.8%           
========================================
  Files             ?     552           
  Lines             ?   62992           
  Branches          ?       0           
========================================
  Hits              ?   35832           
  Misses            ?   23964           
  Partials          ?    3196           
Flag Coverage Δ
aggregator 63.3% <ø> (?)
cluster ∅ <ø> (?)
collector 58.4% <ø> (?)
dbnode 60.5% <0.0%> (?)
m3em 46.4% <ø> (?)
metrics 19.7% <60.0%> (?)
msg 74.3% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9778df4...521e70e. Read the comment docs.

@@ -248,6 +254,12 @@ func (c *coordinator) WaitForShardsReady() error {
}

func (c *coordinator) Close() error {
if c.embedded {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be nice if this just handled double closes sanely regardless of embedded or not.

@jaredjenkins jaredjenkins self-requested a review September 28, 2021 15:57
Passing the interrupt channel to multiple goroutines could
cause a race where the main thread ends up missing the interrupt
that triggers a server shutdown. This commit ensures that only
a single goroutine is listening for the interrupt at a given
time and all other interested parties can check the interrupted
channel. The interrupted channel will be closed as soon as
an interrupt is received. Since closed channels return immediately,
this allows any interested goroutine to know if it should terminate
by simply checking the interrupted channel.
This commit fixes a couple small things re: embedded coordinators:

1) Fails fast if dbnode is not started when creating an embedded
coordinator.

2) Adds support for port and filepath replacement in embedded
config.
Base automatically changed from nb/girl-interrupted to master September 28, 2021 16:57
@nbroyles nbroyles merged commit 6ead62f into master Sep 28, 2021
@nbroyles nbroyles deleted the nb/dbnode-started branch September 28, 2021 17:22
Antanukas pushed a commit that referenced this pull request Oct 5, 2021
This commit fixes a couple small things re: embedded coordinators:

1) Fails fast if dbnode is not started when creating an embedded
coordinator.

2) Adds support for port and filepath replacement in embedded
config.
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.

2 participants