shinken-monitoring / mod-livestatus

Shinken module for presenting data with a MK/Livestatus comptabile interface
GNU Affero General Public License v3.0
15 stars 20 forks source link

Fix wait query bug #43

Closed gst closed 9 years ago

gst commented 9 years ago

NOT (because it was already buggy) shamefully introduced by myself in d61b659345385386ba8d3fff1158fe0923ae7ab8.
(but there was no test case covering the missed thing :p)

and while at it : simplified and clarified handling of multiple components in a single request. (mostly DRY).

Seb-Solon commented 9 years ago

Ok If I got it well :

Regarding the database open, I'm not sure. Is this mean that the open fucntion was never called explicitely? Because here you only open it in a specific part of the code.

Edit : I hope the patch will be back portable on 1.4 branch.

gst commented 9 years ago

Ok If I got it well :

you all got it well.

For the database open : it's also already open in others code paths effectively. but would need to double check that these code paths actually really don't need to be open (like: when livestatus is started the db sqlite can be rotated/cleaned or things like that).

It should be back-portable to 1.4 without great difficulties. I hope so :p

(I also hope that we'll sooner than later be able to drop that 1.4 support ;) )

greg.

----- Mail original ----- De: "Sébastien Coavoux" notifications@github.com À: "shinken-monitoring/mod-livestatus" mod-livestatus@noreply.github.com Cc: "Grégory Starck" gregory.starck@savoirfairelinux.com Envoyé: Lundi 22 Décembre 2014 04:33:02 Objet: Re: [mod-livestatus] Fix wait query regression (#43)

Ok If I got it well :

* Wait query were not parsed correctly since the rework. 
* You fixed this issue with the handle_wait_query() 
* You added a test case for wait query (there was not wait queries before??) 
* You reworked a part of the multiple request type with the _is_valid_queries 
* You added a Livestatus exception for invalid headers 

Regarding the database open, I'm not sure. Is this mean that the open fucntion was never called explicitely? Because here you only open it in a specific part of the code.

— Reply to this email directly or view it on GitHub .

Grégory Starck

Ingénieur logiciel Savoir-faire Linux Email: gregory.starck@savoirfairelinux.com Tél: +1 514 276 5468 Poste #176

gst commented 9 years ago

currently trying the back-port to 1.4 ..

gst commented 9 years ago

This is required for the test to pass :

1) https://github.com/naparuba/shinken/pull/1426 2) https://github.com/shinken-monitoring/mod-logstore-sqlite/pull/10

gst commented 9 years ago

f** checkmk + livestatus, fUc* livestatus

naparuba commented 9 years ago

good resume of the whole LS+multisite thing :)

On Mon, Dec 22, 2014 at 11:19 PM, Grégory Starck notifications@github.com wrote:

f** checkmk + livestatus, fUc* livestatus

— Reply to this email directly or view it on GitHub https://github.com/shinken-monitoring/mod-livestatus/pull/43#issuecomment-67897635 .

naparuba commented 9 years ago

question: when are waitquery used in multisite? because I think such queries can just break my worker idea in fact :( :(

Is it possible to just drop them and hope multisite to still work?

On Wed, Dec 24, 2014 at 10:56 AM, nap naparuba@gmail.com wrote:

good resume of the whole LS+multisite thing :)

On Mon, Dec 22, 2014 at 11:19 PM, Grégory Starck <notifications@github.com

wrote:

f** checkmk + livestatus, fUc* livestatus

— Reply to this email directly or view it on GitHub https://github.com/shinken-monitoring/mod-livestatus/pull/43#issuecomment-67897635 .

gst commented 9 years ago

good resume of the whole LS+multisite thing :)

I was a bit rude ;)

question: when are waitquery used in multisite?

good question.. I mostly don't know (yet) :s

Is it possible to just drop them and hope multisite to still work? because I think such queries can just break my worker idea in fact :( :(

also good question ;) but don't worry, there aren't problems, there are solutions and we'll found what we need in any case ;)

gst commented 9 years ago

Here it is :)

With this, Check_Mk is happy/ok when forcing an immediate host check and waiting for its result :) (and normally nothing is broken, let's be confident in that.. ho well ;) )

Happy Christmas !

gst commented 9 years ago

question: when are waitquery used in multisite?

already when you do a "Reschedule an immediate check of this host".

in check_mk code :
-> ajax_action() function -> action_reschedule().

Are you ok to merge Nap ?

gst commented 9 years ago

NB: I began backporting to 1.4 it ; won't be as easy/direct as I would have liked but I'll get it done ;)

gst commented 9 years ago

Backporting correctly ongoing, nearly done :)

gst commented 9 years ago

At least it's already good with Travis :

https://travis-ci.org/gst/shinken/builds/46213462

But with jenkins and "long run" tests it's different.. have to backport some others things and adapt more the tests unfortunately - actually.. I'm continuing a bit to see if I get something good / acceptable..

gst commented 9 years ago

for info, first results with jenkins :

https://test.savoirfairelinux.com/view/Shinken/job/Shinken-with-jenkins/186/consoleFull

but since then I'm ongoing and have changed/backported already many/others things..