MetPX / sarracenia

https://MetPX.github.io/sarracenia
GNU General Public License v2.0
46 stars 22 forks source link

Queue Name order is important in 3.0.56rc1 ... was not before... changes things. #1267

Closed petersilva closed 4 weeks ago

petersilva commented 1 month ago

Stuff done for subscriptions.json seems to have made queue name resolution janky.

petersilva commented 1 month ago

@andreleblanc11 I don't remember how to reproduce this... do you?

andreleblanc11 commented 1 month ago

It seems to be happening when we have a configuration that is restarted and cleaned up from V 3.00.55post1 to 3.00.56rcX.

The only component we've seen affect so far is a sarra. The queueName resolves wrong in sr3 show but is fine in the config and in the *.qname file

grep queueName sarra/get-lrgs-data2.conf
queueName q_tsub.${PROGRAM}.${CONFIG}.ddsr-dev

sr3 show sarra/get-lrgs-data2
'queueName': 'q_tsub.sarra.get-lrgs-data2.sarra_ddsr-cmc-dev02.cmc.ec.gc.ca_40812612
'queueName_resolved': 'q_tsub.sarra.get-lrgs-data2.sarra_ddsr-cmc-dev02.cmc.ec.gc.ca_40812612

sr3r 'cat ~/.cache/sr3/sarra/get-lrgs*/*.qname  && printf "\n"'
ddsr-cmc-dev01: q_tsub.sarra.get-lrgs-data2.ddsr-dev
ddsr-cmc-dev02: q_tsub.sarra.get-lrgs-data2.ddsr-dev
petersilva commented 1 month ago

OK. I understand what is going on... it's an order thing.


broker
exchange 
subtopic

queueName

The queueName gets resoved when you hit subtopic before you know the queueName, so it always gets the default. Moving queueName above the subtopic makes everything consistent.


broker
exchange 
queueName
subtopic

so that's a work-around... to fix the problem when you run into it... is it a bug... hmm... have to think some more.

petersilva commented 1 month ago

removing the bug tag. Should document that queueName... if you are going to customize it should come before a subtopic where you want the queuename used.

petersilva commented 1 month ago

So... This is probably a good change (e.g. the configuration files really need to change.) because in the future the idea is to support multiple queues and multiple bindings with a single subscriber... the implementation of subscriptions.json is actually a step on the way to that. It is not completely implemented, but once the features (coming in 57? or so... a few weeks?) will mean you could do something like:


queueName q_${BROKER_USER}_lo_pri

subtopic a
subtopic b

queueName q_${BROKER_USER}_hi_pri

subtopic c
subtopic d

This would allow a single subscriber to have a whole bunch of low pri stuff in one declared queue, and a (hopefully smaller) bunch of stuff going into the hi priority queue. since we gather from both queues, the high priority stuff should get through faster.

There should also be the ability to specify multiple brokers to do winnowing with a single subscriber, simplifying a lot of configurations. This is all planned stuff for the future...

It isn't complete for this release... the stub of putting stuff in subscriptions.json is just a part of what is needed. but that means that whenever you hit a subtopic verb , you evaluate the queueName setting...

So... It's not a bug, it's a change in the grammar that we probably need to adjust the configs to.

petersilva commented 1 month ago

Proposed documentation change... I guess will do French if this is OK.

andreleblanc11 commented 1 month ago

We will need to be careful when publishing this next release into operations. Lots of our configurations have subtopic set before queueName as it wasn't a requirement to have this ordering before.

I think it would also be a good idea to add a WARNING (maybe even a ERROR?) message in the logs if ever subtopic is set before queueName in a configuration.

petersilva commented 1 month ago

I think we should just fix all our configurations before we update. In the current/older versions... it doesn't matter where queueShare/queuenName are, so it is harmless to change them all.

The problem is queueName is a thing that analysts override a lot... but most people never use... It has a good default, so there is no easy way to detect the error... maybe the user WANTED the default value (90% case.)

petersilva commented 1 month ago

also... if the configs don't change this time... they will need to change next release anyways...

petersilva commented 1 month ago

Discussed adding some logic to "move" queueName ahead of subtopic when running sr3 convert ... hmm: