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

Fix write fanouts with aggregated namespaces #991

Merged
merged 12 commits into from
Sep 30, 2018

Conversation

richardartoul
Copy link
Contributor

No description provided.

echo "Wait until data begins being written to remote storage for the aggregated namespace"
ATTEMPTS=10 TIMEOUT=2 retry_with_backoff \
'[[ $(curl -sSf 0.0.0.0:9090/api/v1/query?query=database_write_tagged_success\\{namespace=\"agg\"\\} | jq -r .data.result[0].value[1]) -gt 0 ]]'

Copy link
Collaborator

Choose a reason for hiding this comment

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

remove the docker compose down below

@@ -11,6 +11,13 @@ echo "Run m3dbnode and m3coordinator containers"
docker-compose -f ${COMPOSE_FILE} up -d dbnode01
docker-compose -f ${COMPOSE_FILE} up -d coordinator01

# think of this as a defer func() in golang
function defer {
Copy link
Collaborator

Choose a reason for hiding this comment

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

fancy!

@@ -6,9 +6,43 @@ echo "Bringing up nodes in the backgorund with docker compose, remember to run .
docker-compose -f docker-compose.yml up -d --renew-anon-volumes
echo "Nodes online"

sleep 10
Copy link
Collaborator

Choose a reason for hiding this comment

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

drop changes to m3_stack from the pr

curl -vvvsSf -X DELETE 0.0.0.0:7201/api/v1/namespace/default
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: newline

@@ -359,7 +360,7 @@ func newDownsampler(
RulesKVStore: kvStore,
AutoMappingRules: autoMappingRules,
ClockOptions: clock.NewOptions(),
InstrumentOptions: instrumentOpts,
InstrumentOptions: instrumentOpts.SetMetricsScope(tally.NoopScope),
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: put a reference to 992

TODO: remove after https://github.com/m3db/m3/issues/992 is fixed

@@ -58,6 +58,7 @@ import (
"github.com/m3db/m3x/pool"
xsync "github.com/m3db/m3x/sync"
xtime "github.com/m3db/m3x/time"
"github.com/uber-go/tally"
Copy link
Collaborator

Choose a reason for hiding this comment

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

metalinter is going to complain about this

@codecov
Copy link

codecov bot commented Sep 30, 2018

Codecov Report

Merging #991 into master will increase coverage by 21.9%.
The diff coverage is 16.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #991      +/-   ##
==========================================
+ Coverage   55.91%   77.82%   +21.9%     
==========================================
  Files         407      411       +4     
  Lines       34318    34516     +198     
==========================================
+ Hits        19188    26861    +7673     
+ Misses      13322     5783    -7539     
- Partials     1808     1872      +64
Flag Coverage Δ
#dbnode 81.37% <ø> (+11.97%) ⬆️
#m3ninx 75.25% <ø> (+21.18%) ⬆️
#query 64.37% <16.66%> (+64.37%) ⬆️
#x 84.72% <ø> (+23.61%) ⬆️

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 22b0b4c...a6dd6a4. Read the comment docs.

Copy link
Collaborator

@prateek prateek left a comment

Choose a reason for hiding this comment

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

LGTM once build passes

@richardartoul richardartoul merged commit 0837b49 into master Sep 30, 2018
@prateek prateek deleted the ra/fix-coord-dual-write branch September 30, 2018 05:07
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