Single queue #12
Single queue #12
Conversation
README.md
Outdated
| @@ -144,6 +144,7 @@ The default location for broker configuration file is | |||
| `err`, `warn`, `notice`, `info` and `debug` | |||
| - _max-size_ -- maximal size of log file before rotating | |||
| - _rotations_ -- number of rotation kept | |||
| - _queue_manager_ -- selection of the queue manager implementation responsible for assigning jobs to workers. Currently only `single` (the default) and `multi` queue managers are in production version. Single-queue manager has one queue and dispatches jobs on demand as workers become available. Multi-queue manager has a queue for every worker, jobs are assigned immediately and cannot be re-assigned unless failure occurs. I.e., `single` provides better load balancing, `mutli` has lower dispatching overhead. | |||
| @@ -127,7 +128,17 @@ void broker_core::broker_init() | |||
| logger_->info("Initializing broker connection..."); | |||
| workers_ = std::make_shared<worker_registry>(); | |||
| context_ = std::make_shared<zmq::context_t>(1); | |||
| queue_ = std::make_shared<multi_queue_manager>(); | |||
|
|
|||
| // Yes, this may be done better, but it will do for now. | |||
There was a problem hiding this comment.
If I were pedantic, I would ask you to elaborate on what's the better way. But I'm not, so keep your secrets if you want.
There was a problem hiding this comment.
You can create a static map<string, obj>, where the obj is either factory, prototype, or singleton. This map can be then filled by static constructors (so the "registration" for translation is done along with the declaration and will not be forgotten when extensions are implemented). However, for 2 or 3 manager classes, it does not worth implementing.
There was a problem hiding this comment.
heh, that sounds like so much of an overkill I'd even remove the comment
There was a problem hiding this comment.
Well, I have a personal grudge against extensive if-else branching, so I shall keep it :)
src/queuing/single_queue_manager.h
Outdated
| return a.arrived_at < b.arrived_at; | ||
| } | ||
|
|
||
| void drop_caches() {} |
There was a problem hiding this comment.
I believe we can safely remove this - it was there to speed up the simulation of another comparator (OAGM, if I recall correctly), which wasn't that great anyways.
| if (!is_request_assignable(job.request)) { | ||
| // the job cannot be accomodated anymore... | ||
| result->push_back(job.request); | ||
| job.request = nullptr; // mark the job for removal |
There was a problem hiding this comment.
That's... unusual. In such short code though... I guess it's alright
There was a problem hiding this comment.
What is unusual? The marking for deletion? If you have a better way, I would be happy to entertain it, but I am quite sure that deleting items from a container in a for-each loop (over that container) is not something you want to do...
There was a problem hiding this comment.
If I went with the "mark for deletion" route, I'd probably make a separate collection of stuff to remove. As I said, the current implementation works just fine and it's unlikely the method will ever grow enough for it to become unmaintainable.
An alternative idea I have is to use stable_partition to push the unsatisfiable request to the edge of the queue and then snip them off.
There was a problem hiding this comment.
that's what remove_if does ... I guess I have become too biased towards functional JS-like solutions ;)
There was a problem hiding this comment.
well, you can call mutating the queue items in place many names, but functional isn't one of them 🙂
There was a problem hiding this comment.
Yep, I mutated the objects inside, but the collection was intact until filtered by erase-remove, which is functional (that's what I meant).
krulis-martin
force-pushed
the
single-queue
branch
from
ed388f9
to
31fe7fa
Compare
Codecov Report
@@ Coverage Diff @@
## master #12 +/- ##
==========================================
+ Coverage 79.78% 81.10% +1.32%
==========================================
Files 38 40 +2
Lines 1385 1535 +150
==========================================
+ Hits 1105 1245 +140
- Misses 280 290 +10
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
No description provided.