Bug 41254 - apr_queue_t enhancements
Summary: apr_queue_t enhancements
Status: NEW
Alias: None
Product: APR
Classification: Unclassified
Component: APR-util (show other bugs)
Version: HEAD
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: Apache Portable Runtime bugs mailinglist
URL:
Keywords: PatchAvailable
: 41099 (view as bug list)
Depends on:
Blocks:
 
Reported: 2006-12-28 22:11 UTC by Yossi Neiman
Modified: 2009-06-29 09:38 UTC (History)
1 user (show)



Attachments
Diff for apr_queue.c (8.04 KB, patch)
2006-12-28 22:17 UTC, Yossi Neiman
Details | Diff
Diff for apr_queue.h (1.85 KB, patch)
2006-12-28 22:18 UTC, Yossi Neiman
Details | Diff
**Updated** Diff for apr_queue.c (7.88 KB, patch)
2007-01-03 10:03 UTC, Yossi Neiman
Details | Diff
Patch for apr_queue.h (1.81 KB, patch)
2007-01-03 10:04 UTC, Yossi Neiman
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yossi Neiman 2006-12-28 22:11:13 UTC
I added the ability to manually block/unblock push and pop (independently of one
another), and provided a function to block until the queue is empty.
Comment 1 Yossi Neiman 2006-12-28 22:17:31 UTC
Created attachment 19321 [details]
Diff for apr_queue.c
Comment 2 Yossi Neiman 2006-12-28 22:18:01 UTC
Created attachment 19322 [details]
Diff for apr_queue.h
Comment 3 Yossi Neiman 2006-12-28 22:20:47 UTC
There we go, fixed up the diffs for this.  Hopefully the project will deem this
a useful addition.
Comment 4 Yossi Neiman 2007-01-03 10:03:09 UTC
Created attachment 19345 [details]
**Updated** Diff for apr_queue.c

Fixed the lack of initialization as pointed out by Ryan Phillips.  Performed
patch creation in a way so as to not require -p6 to patch the original source.
Comment 5 Yossi Neiman 2007-01-03 10:04:33 UTC
Created attachment 19346 [details]
Patch for apr_queue.h

Performed patch creation in a way so as to not require -p6 to patch the
original source.  (Matches same -p parameter from apr_queue.c.diff)
Comment 6 Neil Conway 2009-06-28 23:19:22 UTC
*** Bug 41099 has been marked as a duplicate of this bug. ***
Comment 7 Neil Conway 2009-06-28 23:22:00 UTC
Why is the ability to manually block pushs and pops useful? Seems like a fairly esoteric requirement to me.
Comment 8 Yossi Neiman 2009-06-29 08:53:19 UTC
It suppose it can seem esoteric.  However, specific case scenario where I used this feature was this:

1.  You have an apr_queue_t full of an unknown number of objects to be processed.
2.  Processing of these objects depends on a user-provided configuration (for example credentials to connect to an RDBMS).
3.  The user needed to make a change to the configuration, and wants the program to reload the configuration.
4.  Without the ability to pause the apr_queue_t, we could have a scenario where the config is being reloaded at the exact second one of these objects is being processed, and cause any number of problems (object not getting recorded or program crashing).

There can be other instances where this might be handy as well, such as where you need to pause the queue to do some sort of maintenance and then unpause it.  You can see the old mod_cdr from the FreeSWITCH project to see how I had implemented it if you like.

Since it doesn't add much complexity, it seemed to be something that other folks might find useful if it's available.  Feel free to merge this patch.
Comment 9 Neil Conway 2009-06-29 09:04:06 UTC
You could also achieve this functionality by inserting a "configuration change request" into the queue itself, and having the queue consumer pause and reload the configuration when it pops the change request from the queue. If you need to be notified when the queue consumer has applied the conf change, you can put a condition var in the change request message and wait for the consumer to signal it.

This approach also has the advantage of not blocking concurrent insertions, which reduces throughput.
Comment 10 Yossi Neiman 2009-06-29 09:10:19 UTC
It's been over 2 years since I wrote this patch, and while what you said is definitely another option of how to handle the reload scenario, I thought that I wrote my code so that if you're blocking pop it shouldn't block pushes.  I thought I made it fine-grained enough that you could block either pushes or pops independently.

I won't be able to re-apply this patch to be 100% certain until later on today though.  Please feel free to correct me if I'm wrong.
Comment 11 Neil Conway 2009-06-29 09:19:06 UTC
Oh, right -- I was assuming the configuration change would need to be applied by the consumer thread itself. If that's not the case, you could use the "block pop" feature to block the consumer from doing any additional work, and then apply the configuration change from another thread while the consumer is blocked. However, AFAICS you'd still need some other mechanism to avoid a race condition: if the consumer pop()s an item just before the block_pop() arrives, it could still be processing that item while the configuration change is being applied.
Comment 12 Yossi Neiman 2009-06-29 09:38:01 UTC
Correct, this was designed for a situation where the consumer thread is not the one initiating any reload of configuration.  Also, it would need to be up to the application that is consuming off of the apr_queue_t to determine how to handle the potential for that specific race condition, and not the queue itself.