Pluggable cache system?

[jimsmart]

Hi,

Thanks for Colly! :)

I have a task with hundreds of thousands of pages, so obviously I am using Colly's caching, but it's basically 'too much' for the filesystem. (Wasted disk space, a pain to manage, slow to backup, etc)

I'd like to propose a pluggable cache system, similar to how you've made other Colly components.

Perhaps with an API like this:-

type Cache interface {
	Init() error
	Get(url string) (*colly.Response, error)
	Put(url string, r colly.Response) error
	Remove(url string) error
}

...or...

type Cache interface {
	Init() error
	Get(url string) ([]byte, error)
	Put(url string, data []byte) error
	Remove(url string) error
}

The first one won't be possible if you then wish to implement FileSystemCache in a subpackage to Colly though.

The reason I also need a Remove method is because one project has a site that sometimes serves maintenance pages, and whilst I can detect these, Colly currently has no method of saying stop, or of rejecting a page after processing. Obviously, the last thing I want part way through a crawl is to have my cache poisoned. But that's probably a separate issue, that I can live with if I can do the removal of bad pages myself.

If pluggable caches were to be implemented, I have existing code from another project that has a cache built using SQLite as a key-value store, compressing/decompressing the data with Zstandard (it's both surprisingly quick and super efficient on disk space), that I would happily port over. This can either become part of Colly, or a separate thing on my own Github.

I did start implementing this myself, but ran into a problem with how I went about it. (I followed the pattern you have established of having the separate components as subpackages, I then got bitten because my FileSystemCache couldn't easily reach into the Collector to get the CacheDir, I was trying to preserve existing behaviour / API compatibility. Maybe that's not an issue. Maybe these bits shouldn't be subpackages. Obviously once I started thinking things like that I figured it was best to discuss before/instead of proceeding any further.)

— Your thoughts?

[jimsmart]

Another option is to pass/return io.Reader across the interface, instead of []byte

[asciimoo]

@jimsmart thanks for this great idea and for the detailed and clear specification.

I'm currently working on a storage interface (04d96ab) which follows pretty much the same concept. It allows us to use any kind of backend to store visited urls and cookies (http://go-colly.org/docs/best_practices/storage/).
Let me describe its architecture, perhaps it can help to design a more sophisticated cache functionality.
storage interface is a stand alone module in colly: https://github.com/gocolly/colly/blob/master/storage/storage.go and colly imports it. storage module also contains the default naive implementation of the storage interface to be backward compatible. I decided to create separate packages for different implementations of the Storage interface to reduce colly's dependencies. Redis storage implementation can be found here: https://github.com/gocolly/redisstorage .

I think, a similar concept could work with the 2nd cache interface from your specification. What do you think?

Also, I'd suggest an additional Close() error function in the interface to be able to terminate connections.

[jimsmart]

Yes. That's what I was thinking, similar to package storage.

And have a SetCache(c cache.Cache) error method on Collector.

I don't need Close() myself... Get(), Put() and Remove() should all be atomic operations, and each method would simply obtain a database connection on execution, and close it before returning. All Go db drivers use a connection pool behind the scenes anyhow, so it's a good pattern to follow, has minimal overheads and is easy to reason about. It's better to do that than to have an open db connection hanging about for hours/days (it's better to release it back to the pool after every use, that way the driver can deal with broken or stale connections, etc.). — My Init() method here wasn't for creating a connection, it was to create the db / db table if it doesn't already exist (or, for the file system: create the cache dir).

Your redisstorage.Storage could then also implement Get(), Put() and Remove() and would satisfy both storage.Storage and cache.Cache interfaces.

— Slight aside: I see you have a lot of stutter in your interface names. It might be better to move (or perhaps copy) them up to the parent package, then you'd have colly.Storage as the interface in the method calls.

[asciimoo]

And have a SetCache(c cache.Cache) error method on Collector.

Yes.

I don't need Close() myself... Get(), Put() and Remove() should all be atomic operations

You're right, i've removed it from Storage too, thanks.

Your redisstorage.Storage could then also implement Get(), Put() and Remove() and would satisfy both storage.Storage and cache.Cache interfaces.

Storage is responsible for storing both visited urls and cookies, so a simple Get(), Put() isn't enough in this case. Also, it is planned to add robots txt handling to the Storage interface which makes the interface even more complex.

Slight aside: I see you have a lot of stutter in your interface names. It might be better to move (or perhaps copy) them up to the parent package, then you'd have colly.Storage as the interface in the method calls.

I agree, it is a bit redundant, but the package doesn't use internals of the colly package which is quite big. I'm not against moving Storage/Cache interface to colly, especially because it would allow us to use the first proposed interface.

[jimsmart]

Storage is responsible for storing both visited urls and cookies, so a simple Get(), Put() isn't enough in this case.

Of course not! :) I wasn't suggesting it would be, I was suggesting that specific implementations of Storage could (optionally) also implement the cache methods (Get/Put/Remove) as well as the methods on the Storage interface — this would then give the user the option of using that store as both a cache and a store for state persistence, or just one or the other.

[vosmith]

@jimsmart what exactly are you get/putting in the cache? My imagination is having a hard time going beyond image and css files which aren't downloaded through Colly in any case. If it's just HTML isn't the IsVisited feature enough? I was initially excited about the idea of a cacheing backend as well, but I couldnt find a clear enough use case.

[jimsmart]

I have several hundred thousand web pages I am processing to extract data.

Currently, if the task fails part way through, I just resume later, and rely on the cached data (in the current file system cache) to get reprocessed and then it simply resumes from where it got to.

But there are so many files, and most are hundreds of kb each. The cache folder is >6gb after a crawl. Compressed and in Sqlite, that same data is a single file, of <1.5gb. It's fast. It's easy to manage.

I've pushed two repos with working prototype code, run go test in each project folder to execute the tests.

https://github.com/jimsmart/collysqlite - implements a cache using Sqlite.
https://github.com/jimsmart/collyzstandard - implements a compressing cache wrapper using Zstandard.

Both of these use []byte for the type of data being cached — it was the simplest solution to get a prototype up and running.

[jimsmart]

Basically, if I'm going to invest the time and resources to download that much data, I'd most certainly like to keep it around, for later reruns.

[jimsmart]

Scrapy, the leading Python web scraper, has pluggable caches. Maybe take a look?

[jimsmart]

FWIW: some log output from collyzstandard tests showing compression stats...

2018/02/12 02:40:51 compressed 12930/5251, ratio 2.46, in 2.757465ms https://google.com/
2018/02/12 02:40:51 decompressed in 91.872µs https://google.com/
2018/02/12 02:40:53 compressed 351734/71301, ratio 4.93, in 78.481158ms https://facebook.com/
2018/02/12 02:40:53 decompressed in 1.093598ms https://facebook.com/
2018/02/12 02:40:54 compressed 148439/29126, ratio 5.10, in 54.710252ms https://twitter.com/
2018/02/12 02:40:54 decompressed in 338.833µs https://twitter.com/

[asciimoo]

@jimsmart impressive! Could you please open a PR?

[jimsmart]

Sure, I'll get something together ASAP, I'm just on the tail-end of a bit of a coding marathon, so it won't be today.

I don't think my code will integrate cleanly yet, it needs some more work. But I'm on with that already.

I'm also concerned about 'breaking changes' / whether you have any API compatibility guarantees, or anything similar?

[jimsmart]

In my local repo I also have an implementation of 99% of the 'storage' API... for SQLite.

[asciimoo]

Sure, I'll get something together ASAP, I'm just on the tail-end of a bit of a coding marathon, so it won't be today.
I don't think my code will integrate cleanly yet, it needs some more work. But I'm on with that already.

Awesome, there is no rush. Thanks!

I'm also concerned about 'breaking changes' / whether you have any API compatibility guarantees, or anything similar?

Does this change introduce any backward incompatibility?

[jimsmart]

Um, maybe. But I want to minimise that, obviously. In an ideal world there'd be no breaking changes. That's partly why the code needs more work.

I'm new to the party, so I don't know much about any current plans for features/improvements/etc. So I think a slow and gentle approach to this is probably best.