Skip to content
This repository has been archived by the owner on Sep 2, 2020. It is now read-only.

Cache paths for elastic search store #36

Merged
merged 4 commits into from
Jul 2, 2014
Merged

Cache paths for elastic search store #36

merged 4 commits into from
Jul 2, 2014

Conversation

addisonj
Copy link
Contributor

As pointed out in #35, the elastic search path store has to hit elasticsearch for each metric.

This was pretty bad, and more testing showed that it was a big bottleneck for writes.

This implements a simple cache so that after writing, only one check is needed.

One thing to note, just realized that currently, writing paths to elasticsearch currently isn't tenant aware. This doesn't fix that, but expect a patch soon :)

@tmonk42
Copy link

tmonk42 commented Jun 12, 2014

I think I'm missing something here. I've tried both es-rest and es-native and I never get anything in elasticsearch. I see GET to elasticsearch when using es-rest, which results in a 404 (it's a new stat, so not surprising), but never any PUT/POST to create it.

echo "local.random.diceroll 3 date +%s" | nc <my_ip> 2003

(when using es-rest)
org.apache.http.impl.conn.DefaultClientConnection - Sending request: GET /cyanite_paths/path/local.random.diceroll? HTTP/1.1
org.apache.http.wire - >> "GET /cyanite_paths/path/local.random.diceroll? HTTP/1.1[\r][\n]"
(....)
org.apache.http.impl.conn.DefaultClientConnection - Receiving response: HTTP/1.1 404 Not Found
(....)
org.apache.http.wire - << "{"_index":"cyanite_paths","_type":"path","_id":"local.random.diceroll","exists":false}"

(when using es-native)
DEBUG [2014-06-12 23:04:50,619] main - org.elasticsearch.client.transport - [Haywire] adding address [[#transport#-1][i-b6f174be][inet[/172.31.47.63:9300]]]
DEBUG [2014-06-12 23:04:50,658] main - org.elasticsearch.transport.netty - [Haywire] connected to node [[#transport#-1][i-b6f174be][inet[/172.31.47.63:9300]]]
DEBUG [2014-06-12 23:04:50,699] elasticsearch[Haywire][transport_client_worker][T#1]{New I/O worker #1} - org.elasticsearch.transport.netty - [Haywire] disconnected from [[#transport#-1][i-b6f174be][inet[/172.31.47.63:9300]]], channel closed event
INFO [2014-06-12 23:04:50,703] main - org.elasticsearch.client.transport - [Haywire] failed to get node info for [#transport#-1][i-b6f174be][inet[/172.31.47.63:9300]], disconnecting...
org.elasticsearch.transport.NodeDisconnectedException: [][inet[/172.31.47.63:9300]][cluster/nodes/info] disconnected

@tmonk42
Copy link

tmonk42 commented Jun 13, 2014

I added some debugging code to track down where it was missing the PUT for paths not in elasticsearch. When I build with this change, it starts working... O_o

diff --git a/src/org/spootnik/cyanite/es_path.clj b/src/org/spootnik/cyanite/es_path.clj
index e7fc1bc..15e9bed 100644
--- a/src/org/spootnik/cyanite/es_path.clj
+++ b/src/org/spootnik/cyanite/es_path.clj
@@ -75,6 +75,7 @@
[write-key path-exists? tenant path]
(let [paths (es-all-paths path tenant)]
(dorun (map #(if (not (path-exists? (:path %)))

  •               (debug "NH 3 iffy iffy if")
                (write-key (:path %) %)) paths))))
    

    (defn path-exists-cache?

@addisonj
Copy link
Contributor Author

@tmonk42 what clojure and what ES are you using? My clojure isn't super strong, but might make sense if something in there is lazy?

I will double check, but I am running this in prod and am pretty sure it is working.

@tmonk42
Copy link

tmonk42 commented Jun 17, 2014

ES 0.90.12 and built with: Leiningen 2.3.4 on Java 1.6.0_65 Java HotSpot(TM) 64-Bit Server VM

@addisonj
Copy link
Contributor Author

If you have a chance, try with Java 1.7? es-rest should work regardless of
version, es-native probably needs a recent version of 1.1.x

On Tue, Jun 17, 2014 at 2:20 PM, Nathan Haneysmith <notifications@github.com

wrote:

ES 0.90.12 and built with: Leiningen 2.3.4 on Java 1.6.0_65 Java
HotSpot(TM) 64-Bit Server VM


Reply to this email directly or view it on GitHub
#36 (comment).

@tmonk42
Copy link

tmonk42 commented Jun 18, 2014

Rebuilt with Java 1.7 and pointed at Elasticsearch 1.2.1 and it works (and is ridiculously faster). Thanks!

@addisonj
Copy link
Contributor Author

Sweet.

@pyr any chance of getting his merged? Also #37?

@pyr
Copy link
Owner

pyr commented Jun 18, 2014

Yup will do so shortly
On Jun 18, 2014 6:28 PM, "Addison Higham" notifications@github.com wrote:

Sweet.

@pyr https://github.com/pyr any chance of getting his merged? Also #37
#37?


Reply to this email directly or view it on GitHub
#36 (comment).

@chjohnst
Copy link

chjohnst commented Jul 1, 2014

Just tested this patch out, I am seeing around an 8x change in performance. I have a little app I wrote that shoves in 20k metrics as fast a possible continuously and will wait for then to be flushed. I generally see delays as high as 80s periodically waiting for data to be processed/flushed. With the new set of patches I am seeing the first delay around 40s then finally around 10s. Still not 100% though, a normal carbon relay can handle a lot more then this so really hoping we can get to the bottom of some of the slow downs.

@dene14
Copy link

dene14 commented Jul 1, 2014

Tested that PR with my riemann installation. Works damn good, reduced ES load by 1000%
Great work Addison!

pyr added a commit that referenced this pull request Jul 2, 2014
Cache paths for elastic search store
@pyr pyr merged commit 6607d66 into pyr:master Jul 2, 2014
@pyr
Copy link
Owner

pyr commented Jul 2, 2014

thanks addison !

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants