scrapinghub/scrapylib — pull request #27 by kmike (2013-06-10 23:14:30+00:00)
Title: HCF experiments

Hi,

I don't know if this pull request should be actually merged (because the approach to HCFMiddleware is quite different and doesn't have all current features); it is opened more for a code review.

In this pull request Gilberto's HCF code is modified to use a different approach:

* job re-scheduling is removed;
* the number of concurrently processed HCF batches is limited (to 5 by default);
* 'idle_spider' is back, but its purpose is a bit different because of a previous point;
* batches are deleted in 3 cases:

1) when spider finished normally;
2) when spider finished handling of requests from all currently scheduled batches and entered 'idle' state;
3) when spider finished in an unclean fashion and all requests from a batch were handled (batches with remaining requests are not deleted).

So, this introduces request-level tracking. We fetch a batch and track which requests from it are finished (i.e. spider handled them). I had hard time trying to track all cases (e.g. DNS errors can't be tracked outside downloader middleware because there is no signal or other indication for download error in scrapy; redirect middleware is also tricky) before realizing that with limited number of concurrently processed batches "spider_idle" signal provides a nice way to do "checkpoints": if spider is entered "idle" state then even if some of the requests from started batches are not marked as "done", they still can be deleted.

Request tracking puts some private garbage to request.meta. I think that now as we have limited batches concurrency, we can remove request tracking and stop marking batches as completed if spider doesn't finished normally. This will lead to a bigger number of duplicated items (because the whole last chunk of batches will be rescheduled), but as we have intermediate "checkpoints" in idle_spider, the last chunk won't be larger than HCF_MAX_CONCURRENT_BATCHES. Granular tracking could still de desired, so maybe it is better to just make it optional.

There are other API changes:

* target slot and frontier are now set via Request meta arguments (if missing, they equals to spider's incoming slot/frontier);
* incoming slot and frontier are set via spider attributes; by default, input and output frontier is a spider name; slot is just '0' by default;
* with previous 2 changes global HS_SLOT and HS_FRONTIER options are no longer needed (and they are removed);
* I removed HS_NUMBER_OF_SLOTS support for now because it can be easily implemented in user code (by passing different slot values as Request arguments); slot naming could be project-specific, and there is no support for multiple input slots built-in anyway;
* spiders can implement custom Request serialization/deserialization (so non-url fingerprints and custom callbacks could are supported in user code, and use_hcf option is not required).

I don't have a local HubStorage installation, so there are zero new tests, and Gilberto's tests are likely broken now; the code works for me (I tried different things locally), but the first production run using it is not yet finished running.