NSLS-II / nslsii

NSLS-II related devices
BSD 3-Clause "New" or "Revised" License
11 stars 23 forks source link

handle exceptions from Kafka producers #101

Closed jklynch closed 4 years ago

jklynch commented 4 years ago

This PR adds exception handling to Kafka message publishing. The intention is to insulate the RunEngine from Kafka failures.

codecov-commenter commented 4 years ago

Codecov Report

Merging #101 into master will increase coverage by 0.46%. The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #101      +/-   ##
==========================================
+ Coverage   56.61%   57.07%   +0.46%     
==========================================
  Files          13       13              
  Lines         892      897       +5     
==========================================
+ Hits          505      512       +7     
+ Misses        387      385       -2     
Impacted Files Coverage Δ
nslsii/__init__.py 38.02% <66.66%> (+0.04%) :arrow_up:
nslsii/_version.py 44.80% <0.00%> (+1.79%) :arrow_up:

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 18c0632...e0be416. Read the comment docs.

jklynch commented 4 years ago

queued.max.messages.kbytes seems to be the Consumer parameter. I think the analogous Producer parameter is queue.buffering.max.kbytes so I'll set that to 10 times the default. I'll add compression as well.

gwbischof commented 4 years ago

I tested this on cmb, a run with 5000 takes 15s, the same run with kafka subscribed takes 20s. The compression option didn't affect the execution time.

gwbischof commented 4 years ago

Do we need to do anything special with the consumers if we turn on compression?

jklynch commented 4 years ago

I think the consumers know if compression is used.

mrakitin commented 4 years ago

@jklynch said the tests pass locally. Going to merge it now.