-
1. Re: Some comments on new scheduled delivery work
timfox Oct 8, 2008 3:37 AM (in response to timfox)One other thing:
If you're going to add a scheduledDeliveryTime attribute to the ref, then there's no need for a separate addScheduledDelivery method, so they should be combined.
More over the current addScheduledDelivery method bypasses all the message and size counting done in add. -
2. Re: Some comments on new scheduled delivery work
ataylor Oct 8, 2008 4:40 AM (in response to timfox)a) Can you tell your formatter to add a space between if statements and the opening brace?
It already is, I musn't have formatted this piece of code.
anonymous wrote : b) Main issue. Scheduled delivery time is being stored in the journal at the message level. However, it should be an attribute of the message reference, not the message. (See my earlier post)
So at what point do i add a record to the journal then? I guess i just need to add the queue id and the message id to the encoding.
anonymous wrote : ) Can you point me to the core tests for this functionality?
theres a ScheduledMessageTest under the timing tests. Ive also added some tests to unut tests that already exist, journalManagerTest etc
If you're going to add a scheduledDeliveryTime attribute to the ref, then there's no need for a separate addScheduledDelivery method, so they should be combined.
More over the current addScheduledDelivery method bypasses all the message and size counting done in add.
Ok i'll sort -
3. Re: Some comments on new scheduled delivery work
timfox Oct 8, 2008 4:43 AM (in response to timfox)"ataylor" wrote:
So at what point do i add a record to the journal then? I guess i just need to add the queue id and the message id to the encoding.
Yes, the same as is currently done for acknowledgements, which are also at the message reference level as opposed to the message level. -
4. Re: Some comments on new scheduled delivery work
timfox Oct 8, 2008 4:52 AM (in response to timfox)It would be a good idea to add tests for routing a message to multiple queues, and testing that redelivery delay is handled separately for each reference.
-
5. Re: Some comments on new scheduled delivery work
timfox Oct 8, 2008 4:59 AM (in response to timfox)Don't worry too much about unit tests, more important is to have the core functionality tested at the core level.
I think these are currently missing. -
6. Re: Some comments on new scheduled delivery work
ataylor Oct 8, 2008 5:01 AM (in response to timfox)It would be a good idea to add tests for routing a message to multiple queues, and testing that redelivery delay is handled separately for each reference.
okDon't worry too much about unit tests, more important is to have the core functionality tested at the core level.
This is ScheduledMessageTest but since it relies on timing i put in under the timing tests. -
7. Re: Some comments on new scheduled delivery work
ataylor Oct 8, 2008 5:04 AM (in response to timfox)I also would like to write a core integration test that forces paging from the off and then force a depage to make sure that the message is reloaded correctly, i could only write a unit test for this, maybe Clebert can help?
-
8. Re: Some comments on new scheduled delivery work
timfox Oct 8, 2008 5:05 AM (in response to timfox)"ataylor" wrote:
This is ScheduledMessageTest but since it relies on timing i put in under the timing tests.
Ok, looking at the test it looks like an integration tests to me, so should live in integration/something.
I'd also add some tests which test basic scheduled functionality, single and multiple subscribers, mixture of scheduled and non scheduled messages.
Mixtures of different scheduled times etc. Take a look at the jms test for ideas.
Timing tests are basically unit tests which have long pauses etc in them, not integration tests. -
9. Re: Some comments on new scheduled delivery work
timfox Oct 8, 2008 5:06 AM (in response to timfox)"ataylor" wrote:
I also would like to write a core integration test that forces paging from the off and then force a depage to make sure that the message is reloaded correctly, i could only write a unit test for this, maybe Clebert can help?
Why could you only write a unit test for this? You can start the server with low values of paging surely. -
10. Re: Some comments on new scheduled delivery work
ataylor Oct 8, 2008 5:16 AM (in response to timfox)Why could you only write a unit test for this? You can start the server with low values of paging surely.
I wasn't sure how to force a depage, I guess i could set the value to the size of the first message sent forcing the second (scheduled) message to be paged. -
11. Re: Some comments on new scheduled delivery work
clebert.suconic Oct 8, 2008 1:13 PM (in response to timfox)I don't know how that would apply to your test.. the the PagingStore has a method to startPaging and startDepage.
But if you are using regular sending of messages in your test, a depage might start sooner, as the size of the page is tested on every ACK. -
12. Re: Some comments on new scheduled delivery work
clebert.suconic Oct 8, 2008 10:15 PM (in response to timfox)Can we avoid having a different packet and method for scheduledDeliveryTime?
I'm also creating a packet for Big chunks... and the combination of this with ScheduledDeliveryTime would be somewhat weird.
Bigchunk & scheduled
BigChunk & nonscheduled
RegularSize & scheduled
RegularSize & nonScheduled
If you (Andy) is done with scheduledDelivery, I could take care of the change (if you agree with the change of course). -
13. Re: Some comments on new scheduled delivery work
ataylor Oct 9, 2008 5:27 AM (in response to timfox)Ive now changed this so the journal stores against the reference. Ive also added some integration tests.