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 optimization in update_many #81

Merged
merged 2 commits into from Aug 26, 2014

Conversation

timob
Copy link
Contributor

@timob timob commented Jun 11, 2014

Current code at line 653 uses dict() to remove duplicates. This works, but it leaves the list of points not in interval order. Later runs of contiguous points are batched together into a packed string so they can be write() at once. But because the list is out of order, this batching work is defeated, leaving many batches with just on point in them.

This fix removes the dict() call and uses last value in run of points with duplicate intervals during the loop that creates the list of packed strings for batching.

@johnseekins
Copy link
Contributor

Wow! That actually improves batching rather well. Thanks!

@johnseekins
Copy link
Contributor

Would it make more sense to generate the len(alignedPoints) before the loop, so you aren't calling len() for every point?
len_aligned = len(alignedPoints)
for i in xrange(0, len_aligned):
if i+1 < len_aligned and ...

@timob
Copy link
Contributor Author

timob commented Jun 12, 2014

Good to hear it improves things. It is better just to call len() once, ive updated the pull request.

@timob
Copy link
Contributor Author

timob commented Jun 14, 2014

I've written a test (gist here: https://gist.github.com/timob/4981bed6f2cc7f08d8a8), which runs whisper_update on current whisper and this fixed whisper with the same 100 metrics (one every minute) as arguments. Using whisper files with same creation options (1 minute intervals) with debug enabled.

Here is the output:

running update_many with:
[(1402781417, 28011), (1402781477, 84437), (1402781537, 95288), (1402781597, 7329), (1402781657, 71200), (1402781717, 9326), (1402781777, 18198), (1402781837, 57495), (1402781897, 35025), (1402781957, 22940), (1402782017, 60367), (1402782077, 14973), (1402782137, 42780), (1402782197, 17002), (1402782257, 29223), (1402782317, 16417), (1402782377, 33109), (1402782437, 3955), (1402782497, 46733), (1402782557, 5057), (1402782617, 27540), (1402782677, 90918), (1402782737, 21336), (1402782797, 65800), (1402782857, 81447), (1402782917, 61310), (1402782977, 51897), (1402783037, 34889), (1402783097, 72440), (1402783157, 39175), (1402783217, 47590), (1402783277, 74022), (1402783337, 98308), (1402783397, 55809), (1402783457, 68966), (1402783517, 27536), (1402783577, 58103), (1402783637, 17217), (1402783697, 24369), (1402783757, 39880), (1402783817, 55357), (1402783877, 84850), (1402783937, 51678), (1402783997, 47164), (1402784057, 13398), (1402784117, 29645), (1402784177, 49354), (1402784237, 45300), (1402784297, 47790), (1402784357, 63111), (1402784417, 68391), (1402784477, 82357), (1402784537, 84748), (1402784597, 56761), (1402784657, 44834), (1402784717, 14870), (1402784777, 42637), (1402784837, 59909), (1402784897, 86579), (1402784957, 77622), (1402785017, 31367), (1402785077, 54859), (1402785137, 85472), (1402785197, 48661), (1402785257, 38404), (1402785317, 99266), (1402785377, 63423), (1402785437, 59842), (1402785497, 65062), (1402785557, 71930), (1402785617, 7617), (1402785677, 38160), (1402785737, 8100), (1402785797, 4053), (1402785857, 71728), (1402785917, 74252), (1402785977, 68723), (1402786037, 82165), (1402786097, 83804), (1402786157, 26802), (1402786217, 48549), (1402786277, 64546), (1402786337, 68048), (1402786397, 78724), (1402786457, 4732), (1402786517, 49705), (1402786577, 87603), (1402786637, 44986), (1402786697, 12740), (1402786757, 68983), (1402786817, 94288), (1402786877, 84499), (1402786937, 65652), (1402786997, 59386), (1402787057, 12479), (1402787117, 91449), (1402787177, 4649), (1402787237, 5590), (1402787297, 37325), (1402787357, 43332)]
current whisper
DEBUG :: READ 16 bytes #1
DEBUG :: READ 12 bytes #2
DEBUG :: READ 12 bytes #3
DEBUG :: WRITE 12 bytes #1
DEBUG :: WRITE 12 bytes #2
DEBUG :: WRITE 12 bytes #3
DEBUG :: WRITE 12 bytes #4
DEBUG :: WRITE 12 bytes #5
DEBUG :: WRITE 12 bytes #6
DEBUG :: WRITE 12 bytes #7
DEBUG :: WRITE 12 bytes #8
DEBUG :: WRITE 12 bytes #9
DEBUG :: WRITE 12 bytes #10
DEBUG :: WRITE 12 bytes #11
DEBUG :: WRITE 12 bytes #12
DEBUG :: WRITE 12 bytes #13
DEBUG :: WRITE 12 bytes #14
DEBUG :: WRITE 12 bytes #15
DEBUG :: WRITE 12 bytes #16
DEBUG :: WRITE 12 bytes #17
DEBUG :: WRITE 12 bytes #18
DEBUG :: WRITE 12 bytes #19
DEBUG :: WRITE 12 bytes #20
DEBUG :: WRITE 12 bytes #21
DEBUG :: WRITE 12 bytes #22
DEBUG :: WRITE 12 bytes #23
DEBUG :: WRITE 12 bytes #24
DEBUG :: WRITE 12 bytes #25
DEBUG :: WRITE 12 bytes #26
DEBUG :: WRITE 12 bytes #27
DEBUG :: WRITE 12 bytes #28
DEBUG :: WRITE 12 bytes #29
DEBUG :: WRITE 12 bytes #30
DEBUG :: WRITE 12 bytes #31
DEBUG :: WRITE 12 bytes #32
DEBUG :: WRITE 12 bytes #33
DEBUG :: WRITE 12 bytes #34
DEBUG :: WRITE 12 bytes #35
DEBUG :: WRITE 12 bytes #36
DEBUG :: WRITE 12 bytes #37
DEBUG :: WRITE 12 bytes #38
DEBUG :: WRITE 12 bytes #39
DEBUG :: WRITE 12 bytes #40
DEBUG :: WRITE 12 bytes #41
DEBUG :: WRITE 12 bytes #42
DEBUG :: WRITE 12 bytes #43
DEBUG :: WRITE 12 bytes #44
DEBUG :: WRITE 12 bytes #45
DEBUG :: WRITE 12 bytes #46
DEBUG :: WRITE 12 bytes #47
DEBUG :: WRITE 12 bytes #48
DEBUG :: WRITE 12 bytes #49
DEBUG :: WRITE 12 bytes #50
DEBUG :: WRITE 12 bytes #51
DEBUG :: WRITE 12 bytes #52
DEBUG :: WRITE 12 bytes #53
DEBUG :: WRITE 12 bytes #54
DEBUG :: WRITE 12 bytes #55
DEBUG :: WRITE 12 bytes #56
DEBUG :: WRITE 12 bytes #57
DEBUG :: WRITE 12 bytes #58
DEBUG :: WRITE 12 bytes #59
DEBUG :: WRITE 12 bytes #60
DEBUG :: WRITE 12 bytes #61
DEBUG :: WRITE 12 bytes #62
DEBUG :: WRITE 12 bytes #63
DEBUG :: WRITE 12 bytes #64
DEBUG :: WRITE 12 bytes #65
DEBUG :: WRITE 12 bytes #66
DEBUG :: WRITE 12 bytes #67
DEBUG :: WRITE 12 bytes #68
DEBUG :: WRITE 12 bytes #69
DEBUG :: WRITE 12 bytes #70
DEBUG :: WRITE 12 bytes #71
DEBUG :: WRITE 12 bytes #72
DEBUG :: WRITE 12 bytes #73
DEBUG :: WRITE 12 bytes #74
DEBUG :: WRITE 12 bytes #75
DEBUG :: WRITE 12 bytes #76
DEBUG :: WRITE 12 bytes #77
DEBUG :: WRITE 12 bytes #78
DEBUG :: WRITE 12 bytes #79
DEBUG :: WRITE 12 bytes #80
DEBUG :: WRITE 12 bytes #81
DEBUG :: WRITE 12 bytes #82
DEBUG :: WRITE 12 bytes #83
DEBUG :: WRITE 12 bytes #84
DEBUG :: WRITE 12 bytes #85
DEBUG :: WRITE 12 bytes #86
DEBUG :: WRITE 12 bytes #87
DEBUG :: WRITE 12 bytes #88
DEBUG :: WRITE 12 bytes #89
DEBUG :: WRITE 12 bytes #90
DEBUG :: WRITE 12 bytes #91
DEBUG :: WRITE 12 bytes #92
DEBUG :: WRITE 12 bytes #93
DEBUG :: WRITE 12 bytes #94
DEBUG :: WRITE 12 bytes #95
DEBUG :: WRITE 12 bytes #96
DEBUG :: WRITE 12 bytes #97
DEBUG :: WRITE 12 bytes #98
DEBUG :: WRITE 12 bytes #99
DEBUG :: WRITE 12 bytes #100
fixed whisper
DEBUG :: READ 16 bytes #1
DEBUG :: READ 12 bytes #2
DEBUG :: READ 12 bytes #3
DEBUG :: WRITE 1200 bytes #1

A whisper-fetch on both the resulting files returns the same data in each interval bucket.

Here as you can see current whisper does 100 writes, the fixed whisper does 1 write.

@steve-dave
Copy link
Member

It may be better to use the approach in #69

Once it is finalised and accepted it could be ported to master.

@obfuscurity
Copy link
Member

Lovely, the output above refs all the other Whisper issues. Please remember to wrap code (or output) blocks in backticks to avoid this in the future.

@obfuscurity
Copy link
Member

@steve-dave @SEJeff Would you mind reviewing #89, #81 and #69 and making an executive decision on which (if any) we should move forward with?

This was referenced Aug 26, 2014
@obfuscurity
Copy link
Member

@steve-dave @SEJeff Disregard previous request. I'm merging this one now for master and will merge #89 into 0.9.x once @deniszh tweaks a couple bits.

obfuscurity added a commit that referenced this pull request Aug 26, 2014
fix write optimization in update_many
@obfuscurity obfuscurity merged commit 964fc1b into graphite-project:master Aug 26, 2014
@obfuscurity
Copy link
Member

Thanks @timob! ✨ 🍰 :shipit:

obfuscurity added a commit that referenced this pull request Aug 27, 2014
@zzl0
Copy link

zzl0 commented Nov 6, 2014

🆒

@drawks
Copy link
Member

drawks commented Nov 6, 2014

Hawt
👍

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.

None yet

6 participants