Colly: memory issue

[damoncoo]

Hi,
I'm using colly for my scraping tools, it is fantastic, but I'm struggling days of memory issue.
Memory continue increasing util running out of it. I capture two images to show this.Please help me out of this.

[MichaelMonashev]

I have the same problem:

(pprof) top
Showing nodes accounting for 358.17MB, 81.73% of 438.26MB total
Dropped 68 nodes (cum <= 2.19MB)
Showing top 10 nodes out of 99
      flat  flat%   sum%        cum   cum%
  113.24MB 25.84% 25.84%   113.24MB 25.84%  bytes.makeSlice
   67.02MB 15.29% 41.13%    67.02MB 15.29%  golang.org/x/net/html.(*Tokenizer).Token
   50.51MB 11.52% 52.65%    50.51MB 11.52%  golang.org/x/net/html.(*parser).addText
   29.40MB  6.71% 59.36%    29.40MB  6.71%  compress/flate.(*dictDecoder).init
      26MB  5.93% 65.30%    26.50MB  6.05%  golang.org/x/net/html.(*parser).addElement
      21MB  4.79% 70.09%    48.50MB 11.07%  net/url.(*URL).ResolveReference
      16MB  3.65% 73.74%    27.50MB  6.28%  net/url.resolvePath
      14MB  3.19% 76.93%       14MB  3.19%  net/url.parse
   12.50MB  2.85% 79.79%    12.50MB  2.85%  strings.(*Builder).WriteString
    8.50MB  1.94% 81.73%    24.50MB  5.59%  github.com/gocolly/colly.NewHTMLElementFromSelectionNode

[asciimoo]

Could you provide an example scraper?

[lightglitch]

@asciimoo just use your basic example with a site with a lot of pages like a store (try it with https://www.bybebe.pt/ for example), for me looks related to the function handleOnHTML

[akacase]

I have a similar issue, let's take a simple Async collector via this code here:

        c := colly.NewCollector(
            colly.CacheDir(prefix+"/cache"),
            colly.IgnoreRobotsTxt(),
            colly.Async(true),
        )

        c.WithTransport(&http.Transport{
            DisableKeepAlives: true,
        })

        extensions.RandomUserAgent(c)

        c.SetRequestTimeout(time.Second * 10)
        c.AllowURLRevisit = false

Let's also constrain the crawler to the domain, so we can default to an infinite MaxDepth

        allowed := []*regexp.Regexp{}
        allowed = append(allowed, regexp.MustCompile(`.*salesforce.*`))
        c.URLFilters = allowed

This should constrain the crawler to only .*salesforce.* related domains, so we can crawl https://salesforce.com. I've tried DomainGlob as well and observed the same behavior. I'd like to count all the URLs on all pages I can find to do analysis afterwards, so I'll use this callback

        c.OnHTML("body", func(e *colly.HTMLElement) {
            s := make([]string, 0)

            e.ForEach("a[href]", func(_ int, e *colly.HTMLElement) {
                link := e.Attr("href")
                absolute := e.Request.AbsoluteURL(link)
                if url, err := url.ParseRequestURI(absolute); err == nil {
                    if !strings.Contains(url.String(), ".jsp") &&
                        !strings.Contains(url.String(), "tel:") &&
                        !strings.Contains(url.String(), "javascript:") &&
                        !strings.Contains(url.String(), "mailto:") &&
                        !lookup.Has(hash) && url.String() != "" {
                        s = append(s, url.String())
                    }
                }
            })
                       for _, v := range s {
                   e.Request.Visit(v)
                      }
})

We seed the crawler and watch it balloon in memory usage

        if err := c.Visit("https://salesforce.com"); err != nil {
            log.Info().Err(err).Msg("couldn't visit url")
        }

        c.Wait()

You can see major negative effect of you're instantiating new crawlers per website, in my particular case, I either have a flat-file of URLs I'd like to crawl or I'm subscribed to a Pub/Sub of some sort and per iteration a new crawler is instantiated and waited upon, per iteration of this for { } memory isn't cleared at all, here's a pprof image to show the 7th iteration, this is with nodefraction=0:

This is with a 4 core box with about 32gbs of ram, it will kill the box before it can complete. I could be using this library incorrectly and please correct me if wrong, also thanks for all your hard work.

[lightglitch]

@akacase we solved our issue using the queue example http://go-colly.org/docs/examples/queue/, because it's not recursive the memory used is in control. You do loose the referral information because each link is open like a new window.

[akacase]

@lightglitch worried that was the case, thanks for the update. I'll rewrite, it's a shame golang doesn't support TCO.

[akacase]

@akacase we solved our issue using the queue example http://go-colly.org/docs/examples/queue/, because it's not recursive the memory used is in control. You do loose the referral information because each link is open like a new window.

@lightglitch -- did you experience a major speed reduction by moving to the queuing model? speed has decreased at least by 10^1

[lightglitch]

@akacase for my use case we didn't do speed comparatives. That was something we didn't needed to take into account.

[lightglitch]

@akacase increasing the number of workers didn't helped?

[akacase]

@akacase increasing the number of workers didn't helped?

@lightglitch It's somewhat noticeable, but nowhere near as performant as the recursive solution above. I've done runtime.NumCPU() and the latest thread count I've used is 32, but it's still quite slow.

[znas-io]

@damoncoo if you're re-using a collector make sure you detach your OnHTML callback after you're done with it and attach a new one when needed.

querySelector := "div table tr td"
defer func() {
   collector.OnHTMLDetach(querySelector)
}()
collector.OnHTML(querySelector, func(e *colly.HTMLElement){
   fmt.Println(e.Text)
})

[lox]

I'm seeing exactly the same thing as the original issue, although I'm not using OnHTML, I'm using OnResponse and manually parsing with goquery. Surely this is a reasonably notable issue? Having to rewrite crawlers as queues is sometimes not possible.

[asciimoo]

Have you tried to use queues?

[lox]

@asciimoo

Having to rewrite crawlers as queues is sometimes not possible.

Yeah! It's really awkward though, as sometimes you don't know what urls you'll add before you begin the crawl (which makes it hard to use queues).

[asciimoo]

You can add urls to the queue dynamically, not just at the beginning of the scraping.

[asciimoo]

The only difference is that you call queue.AddRequest instead of collector.Visit.

[lox]

Hmmm, interesting, perhaps I am doing it wrong, does queue.Run block on the collector running each request? I was seeing my AddRequest calls not having an effect once the queue had emptied. If the first crawl is what populates subsequent url's in the queue, that is a problem.

[asciimoo]

queue.Run blocks while the queue has requests. If the queue is empty and you add new requests you have to call queue.Run again, but this shouldn't be the problem if you add new requests from your callbacks since it guarantees that there is at least one active request.

[asciimoo]

Take a look at the queue example: https://github.com/gocolly/colly/blob/master/_examples/queue/queue.go

[lox]

I'll take another look!

[lox]

Thanks @asciimoo, the key piece I was missing is that you need to AddRequest in OnRequest. That isn't mentioned in the http://go-colly.org/docs/examples/queue/ 🤔

Btw, this issue probably shouldn't be closed should it? There is absolutely unbounded memory usage in standard use of the Collector. This will bite anyone not using queues won't it?

[asciimoo]

the key piece I was missing is that you need to AddRequest in OnRequest

It can be called in any callback.

There is absolutely unbounded memory usage in standard use of the Collector

It only affects long running jobs. In these cases it's recommended to use queues.
UPDATE: async mode can also solve this issue. Big memory consumption is caused by the recursion of callbacks/new requests.

[lox]

It only affects long running jobs.

It's still an open issue though, isn't it? It should be fixable.

[akacase]

@lox should be fixable by eliminating the recursion or implementing some sort of trampoline. Go doesn't support TCO/TCE from a compiler optimization perspective, so it's up to us to unroll it, hence @asciimoo's suggestion to move to a queue, which is more iterative in nature; however they are most definitely slower and not as natural to use, since crawling is literally recursing a giant tree :)