Improved stability/performance for storage #1774

Merged
merged 3 commits into from Jan 10, 2017

Projects

None yet

4 participants

@iksaif
Member
iksaif commented Dec 14, 2016
  • readers: be more resilient to the loss of a single backend
  • storage: try to shuffle nodes
  • finder: avoid calling get_intervals() when listing metrics
iksaif added some commits Sep 23, 2016
@iksaif iksaif readers: be more resilient to the loss of a single backend
If we have multiple backends we sould not raise an exception
when only one of them is down.
c6cf824
@iksaif iksaif storage: try to shuffle nodes
This will prevent us from hammering always the same remote node
first and help spread the load.
02b6098
@obfuscurity
Member

Generally LGTM. What's the advantage to using random.shuffle()?

/cc @DanCech

@iksaif
Member
iksaif commented Dec 14, 2016 edited

The idea of the random.shuffle() is that it changes the first node that get contacted, this help spreading the load, on our setup we found out that one node would timeout way more than the others, and it was always the first node in the list.

Apparently this is breaking a ceres test, I'll fix that early tomorrow.

@DanCech
Contributor
DanCech commented Dec 14, 2016

code changes look reasonable to me, need to figure out the failing test though.

@iksaif iksaif finder: avoid calling get_intervals() when listing metrics
.get_interval() can be expensive with some finders, avoid
calling it when not necessary by:
- making it a property
- not looking at intervals if there is a single node per path
41cc96d
@iksaif
Member
iksaif commented Dec 14, 2016 edited

looks like it calls listdir in ceres less now because it doesn't try to get the intervals even when you don't need them, which is good.

Test fixed now !

@codecov-io

Current coverage is 65.68% (diff: 57.89%)

Merging #1774 into master will decrease coverage by 0.77%

@@             master      #1774   diff @@
==========================================
  Files            55         55          
  Lines          6262       6275    +13   
  Methods           0          0          
  Messages          0          0          
  Branches       1243       1245     +2   
==========================================
- Hits           4162       4122    -40   
- Misses         1877       1936    +59   
+ Partials        223        217     -6   

Powered by Codecov. Last update f7634f3...41cc96d

@iksaif
Member
iksaif commented Dec 24, 2016

@obfuscurity test are fixed, do I get a final LGTM ? :)

@obfuscurity
Member

👍

@iksaif iksaif merged commit 4005a0e into graphite-project:master Jan 10, 2017

1 of 3 checks passed

codecov/patch 57.89% of diff hit (target 66.46%)
Details
codecov/project 65.68% (-0.78%) compared to f7634f3
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment