bwp91 / homebridge-deebot

Homebridge plugin to integrate ECOVACS Deebot devices into HomeKit.
MIT License
64 stars 4 forks source link

DNS flood #142

Closed RaduMC closed 1 year ago

RaduMC commented 1 year ago

What issue do you have? Please be as thorough and explicit as possible.

The plugin is flooding with DNS requests. 716.000 requests for api-app.dc-eu.ww.ecouser.net in the past 90 days Every 2 minutes, 12 simultaneous DNS requests are sent (in the same second).

This is unreasonable.

Details of your setup.

Please paste any relevant logs below.

DNS Server logs

Time        Request             Response    Client
01:37:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:37:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:37:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:37:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:37:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:37:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:37:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:37:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:37:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:37:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:37:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:37:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:35:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:35:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:35:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:35:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:35:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:35:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:35:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:35:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:35:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:35:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:35:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:35:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:33:44    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:33:44    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:33:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:33:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:33:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:33:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:33:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:33:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:33:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:33:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:33:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
01:33:43    api-app.dc-eu.ww.ecouser.net    Processed   localhost (127.0.0.1)
mrbungle64 commented 1 year ago

@RaduMC

Why are 6 requests per minute unreasonable for a server-side application that needs to be connected to the Ecovacs API endpoint?

RaduMC commented 1 year ago

First of all it's 12 per minute.

The ecovacs API url is literally the top request in my DNS Server, taking 23% of all DNS requests. And I have a considerable home network.

It seems to me you need to query the server every 2 minutes. OK, that's fine. But why 12 times simultaneously every 2 minutes? Why not just once every 2 minutes, and use the response for any other function that requires it in the plugin? So yeah, it seems unreasonable.

mrbungle64 commented 1 year ago

@RaduMC

First of all it's 12 per minute.

12 60 24 * 90 = 1.555.200

716.000 / 90 / 24 / 60 = 5,525

The ecovacs API url is literally the top request in my DNS Server, taking 23% of all DNS requests. And I have a considerable home network.

So yeah, it seems unreasonable.

This plugin is still compatible with older models where a lot of polling was needed. Of course, this can be improved, but that requires quite a bit of optimization work to be still compatible with all models.

Maybe you have the possibility to support @bwp91 to do these optimizations 😉

RaduMC commented 1 year ago

12 60 24 * 90 = 1.555.200

Have you considered the plugin wasn't installed for the whole 3 months? Have you looked at the DNS Server logs where I clearly showed 12 requests ever 2 minutes? No...

This plugin is still compatible with older models where a lot of polling was needed.

There's a difference between pooling once every 2 minutes, and 12 times simultaneously every 2 minutes.

Maybe you have the possibility to support @bwp91 to do these optimizations 😉

Wow, you're pulling that card. "If you don't like it, you do it better". It speaks to the quality of your work, be it free or not.

I realise now I'm barking at the wrong tree, since the issue probably comes from ecovacs-deebot.js Thank you for all your work @bwp91 Much appreciated!

It's not the plugin that's unreasonable, it's you @mrbungle64

mrbungle64 commented 1 year ago

@RaduMC

I realise now I'm barking at the wrong tree, since the issue probably comes from ecovacs-deebot.js

ecovacs-deebot.js is a library. The library does not make a single request on its own.

It's not the plugin that's unreasonable, it's you @mrbungle64

Why so unkind?

bwp91 commented 1 year ago

Hi @RaduMC

In response to your concern, I will be adding a polling interval option to the plugin in order to optimise its performance (this will be a setting per device).

However, I would like to reiterate that respectful communication on here between users is so important. This plugin would not be possible without the work of mrbungle in the first place.

The number of calls this plugin makes is down to this plugin and not the library.

RaduMC commented 1 year ago

@bwp91 Thanks for your response.

I also believe in respectful communication, which is why I hope your comment is not entirely addressed to me.

bwp91 commented 1 year ago

Hi @RaduMC

I have just released a new beta version of the plugin which should allow for choosing a polling interval per device, which has the benefits of:

The default value is 0 ie this will disable polling for all devices unless set otherwise, I will make this clear with the release notes for the next version of the plugin.

Details of installing a beta: https://github.com/bwp91/homebridge-deebot/wiki/Beta-Version

bwp91 commented 1 year ago

Also just wanted to go through your comments to @mrbungle64:

There's a difference between pooling once every 2 minutes, and 12 times simultaneously every 2 minutes.

This is the behaviour of the plugin, not the library. As it needs to query a number of different values via polling each time. The library simply offers the different query methods and I take advantage of a number of them.

https://github.com/bwp91/homebridge-deebot/blob/8a1a69192c44525d378dc2201f1c726cfeee7501/lib/platform.js#L818-L841

Wow, you're pulling that card. "If you don't like it, you do it better". It speaks to the quality of your work, be it free or not.

Based on the above, yes, it is me that can perform optimisations for example this disabling of polling for devices that support the 'push' notifications via the library to the plugin.

I realise now I'm barking at the wrong tree, since the issue probably comes from ecovacs-deebot.js Thank you for all your work @bwp91 Much appreciated! It's not the plugin that's unreasonable, it's you @mrbungle64

Nope, the 'issues' are with my plugin and how I have implemented the library from @mrbungle64.


I also believe in respectful communication, which is why I hope your comment is not entirely addressed to me.

So yes if comments like the above towards mrbungle, or indeed any user, are made, then I will make what I think are justified comments.

I think your language was rude and totally uncalled for, especially since this plugin wouldn't be possible without the hard work of mrbungle.


Anyway, back on topic - I would appreciate some feedback on the new polling options and I hope this is something that you find helpful in reducing the number of http calls that are made.

@donavanbecker I think you use this plugin right? are you able to test the new options?