Skip to content
This repository has been archived by the owner on Jan 14, 2018. It is now read-only.

Implement both retry policy and priorization #99

Closed
stephanenicolas opened this issue May 18, 2013 · 12 comments
Closed

Implement both retry policy and priorization #99

stephanenicolas opened this issue May 18, 2013 · 12 comments
Milestone

Comments

@stephanenicolas
Copy link
Owner

Those are the only features that Volley offers and RS doesn't (yet).

http://www.youtube.com/watch?v=yhv8l9F44qo

@rciovati
Copy link
Contributor

For priorization we could implement our custom ThreadPoolExecutor which internally uses a PriorityBlockingQueue (as examplained here). But if someone will use a custom ExecutorService will lose request priority support. What do you think? Which solutions are you thinking about?

@dpsm
Copy link

dpsm commented May 19, 2013

Should the priority order apply to requests across all spice managers within the processor or within each spice manager context avoiding an eager manager from delaying requests from another manager?

@rciovati
Copy link
Contributor

IHMO priority management should be handled by the request processor.

@dpsm
Copy link

dpsm commented May 19, 2013

I agree. It should not be a big deal for an application to manage and define request priorities across it's components.

Makes things easier to implement and the effectiveness of the prioritization will increase doing it at the processor level.

@stephanenicolas
Copy link
Owner Author

I was thinking, originally, to implement it at both spice manager and
request processor level.

I still think that's a good approach as it should not be such a big change
to RS.

@david, I don't see how/why spice manager early ness could be taken into
account.

I plan to release RS tomorrow and start working on this feature right away.

S.
Le 19 mai 2013 19:28, "Riccardo Ciovati" [email protected] a
écrit :

IHMO priority management should be handled by the request processor.


Reply to this email directly or view it on GitHub.

@dpsm
Copy link

dpsm commented May 19, 2013

I guess it does not really apply what I said. If done at both levels the spice manager will prioritize the requests within its "scope" and the processor will prioritize the overall requests from many managers.

I think that it is a pretty good way to go with regards to this feature.

@stephanenicolas
Copy link
Owner Author

Ok, that's fine. If you guys agree, I will try to implement that tomorrow..

We might need beta testers though :)

@dpsm
Copy link

dpsm commented May 19, 2013

Sure! Let me know if you need any help. I might write some test cases for it as well.

How are u planning to shape the priority api. On the spice request class?

@stephanenicolas
Copy link
Owner Author

On a natural ordering of CachedSpiceRequest + delegation to SpiceRequest.

I would define a few constants, I don't like Volley's approach of an enum,
I would prefer an integer. Devs would be more free and the constants could
just be shorteners.

2013/5/19 David Sobreira Marques [email protected]

Sure! Let me know if you need any help. I might write some test cases for
it as well.

How are u planning to shape the priority api. On the spice request class?


Reply to this email directly or view it on GitHubhttps://github.com//issues/99#issuecomment-18122065
.

Stéphane NICOLAS,
OCTO Technology
Développeur & Consultant Android / Java
..........................................................
50, Avenue des Champs-Elysées
75008 Paris
+33 (0)6.26.32.34.09
www.octo.com - blog.octo.com
www.usievents.com
...........................................................

@stephanenicolas
Copy link
Owner Author

The priorization system has been pushed. I had to dig quite deep into the code of ThreadedPoolExecutor and PriorityBlockingQueue to implement and understand tests. But it works in a predictible way now.

http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/6-b14/java/util/concurrent/ThreadPoolExecutor.java

There are some inherent limitations to this mechanism. For instance, the first runnable/callable passed to the executor doesn't go in the queue. Thus the priority mechanism will only apply when tasks are queued, and this happens when the number of current runners is exceeds the number of max thread in the pool size.

It can look like the implementation is not working, but it actually is and tests work at a rate of 100% (hopefully in CI on a slow emulator as well).

This feature gets all its meaning in the case you load A LOT of requests into RoboSpice. For instance, you load images in a listview AND some data from a Pojo. Then you can decide to give a small priority to images, and you will get your Pojo asap.

@rciovati
Copy link
Contributor

I'm glad my idea made sense :)

@stephanenicolas
Copy link
Owner Author

Actually that was exactly the way I did it. :) Still, the spice manager's queue should also take priority into account that would limit the impact of the first N < #core priorisation problem.

And RetryPolicy + back off alg also remain.
(Note to self ;) )

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants