doujiang24 / lua-resty-kafka

Lua kafka client driver for the Openresty based on the cosocket API
BSD 3-Clause "New" or "Revised" License
801 stars 274 forks source link

fix(producer) too many timer #73

Closed mengskysama closed 4 years ago

jeremyjpj0916 commented 4 years ago

@mengskysama @doujiang24 Are the suggested PR enhancements discussed here needed to make this production safe from a producer perspective? At what volume will current implementation produce too many timers?

anhzhi commented 4 years ago

According to this PR and issues like

https://github.com/doujiang24/lua-resty-kafka/issues/22
https://github.com/doujiang24/lua-resty-kafka/issues/66
...

@doujiang24 can you help to confirm my understanding as follows:

  1. There is no need for the logging about "failed to create timer at _flush_buffer" in most situations, especially when only lua-resty-kafka create timers in openresty.
  2. Even logging about "failed to create timer at _flush_buffer" happens, lua-resty-kafka can still work well because no data is dropped from the ringbuffer (if max_buffering is big enougth).
  3. This PR mainly aims to low the frequency for the log, then to impove performance.
anhzhi commented 4 years ago

@doujiang24 In my opinion, the code(https://github.com/doujiang24/lua-resty-kafka/blob/master/lib/resty/kafka/producer.lua#L283)

    if ringbuffer:need_send() then
        _flush_buffer(self)

    elseif is_exiting() and ringbuffer:left_num() > 0 then
        -- still can create 0 timer even exiting
        _flush_buffer(self)
    end

can be written in synchronous-call style and moved in front to line line 281

--- before the lock released
 _flush_unlock(self)

then timer will also be suppressed

doujiang24 commented 4 years ago

reimplemented by https://github.com/doujiang24/lua-resty-kafka/pull/83 thanks for your contribution.