search

yahoo / streaming-benchmarks

Benchmarks for Low Latency (Streaming) solutions including Apache Storm, Apache Spark, Apache Flink, ...
Apache License 2.0
622 stars 294 forks source link

Fix a bug that load-ids always returns nil #14

Closed gyk closed 8 years ago

kishorvpatil commented 8 years ago

LGTM. +1

yahoocla commented 8 years ago

Thank you for submitting this pull request, however I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! :smile:

yahoocla commented 8 years ago

CLA is valid!

gyk commented 8 years ago

Sorry my code is wrong. finally will get called even if file loading fails.

Possible fix:

(defn load-ids []
  (try
    (with-open [campaigns (clojure.java.io/reader "campaign-ids.txt")
                ads (clojure.java.io/reader "ad-ids.txt")]
      (println "Loading Ids.")
      (let [ids {:campaigns (doall (line-seq campaigns))
                 :ads (doall (line-seq ads))}]
        (println "loading done")
        ids))
    (catch FileNotFoundException e
      (println "Failed to load ids from file."))))

And besides that, it might be a bug that do-setup calls write-to-redis without parameter redis-host (https://github.com/yahoo/streaming-benchmarks/blob/6920c90b73e3d05bd9203cfa0f3163abacedeb70/data/src/setup/core.clj#L247).

yeshengm commented 6 years ago

This is really an important issue, leading to unnecessary disk IOs.