Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Currently working without using tc-aws #101

Closed
wants to merge 1 commit into from

Conversation

heynemann
Copy link
Contributor

Currently it is working with thumbor's storages and I ported support for tc_aws and other extensions. I didn't bother to update opencv and graphicsmagick engines until we manage to update those as well.

Other storages like redis and mongo still need to be updated as well.

List of packages installed:

  • remotecv==3.0.0
  • boto==2.49.0
  • tornado-botocore==1.5.0
  • python-dateutil
  • dateutils==0.6.12
  • shortuuid==1.0.8
  • redis==3.5.3
  • raven==6.10.0
  • cairosvg==1.0.22
  • pycurl==7.44.1

List of packages installed with no-deps:

  • tc-aws==6.2.15
  • tc-core==0.4.1
  • tc-shortener==0.2.2

All the other packages were skipped. Still need to work on them, but this image should be usable now. If someone can help me test the AWS parts it would be great.

Btw running locally with result storage enabled I and 1 thumbor process I got:

❯ hit http://localhost:8888/unsafe/500x150/i.imgur.com/Nfn80ck.png
Running 30s test @ http://localhost:8888/unsafe/500x150/i.imgur.com/Nfn80ck.png
  10 threads and 30 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    45.98ms  126.39ms   1.40s    96.35%
    Req/Sec   127.40     17.55   151.00     79.01%
  Latency Distribution
     50%   22.94ms
     75%   23.76ms
     90%   25.06ms
     99%  832.30ms
  36549 requests in 30.03s, 612.14MB read
Requests/sec:   1217.02

With 16 processes (on a 16 core CPU) I got this:

❯ hit http://localhost:8888/unsafe/500x150/i.imgur.com/Nfn80ck.png
Running 30s test @ http://localhost:8888/unsafe/500x150/i.imgur.com/Nfn80ck.png
  10 threads and 30 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    20.15ms   92.87ms   1.24s    97.53%
    Req/Sec   516.36    343.38     1.41k    70.88%
  Latency Distribution
     50%    5.44ms
     75%   10.01ms
     90%   16.43ms
     99%  617.29ms
  149843 requests in 30.04s, 2.45GB read
Requests/sec:   4987.56

(both done using your docker image)

You may say using result storage is cheating, but I'd argue that's the beauty of Thumbor's architecture. It does not depend on the server being incredibly fast (since we need to do things that are CPU intensive like face detection), but it does scale fairly well given the different layers of storage and result storage.

For more information on what changed you can check the release at https://github.com/thumbor/thumbor/releases/tag/7.0.0

@heynemann
Copy link
Contributor Author

@gingerlime if you could take a look it would be incredible!

@@ -0,0 +1,3 @@
envtpl==0.6.0
circus==0.15.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heynemann I thought we no longer need circus for thumbor 7.0 ?

also can you help me understand the difference between lib-requirements and proc-requirements ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to keep the same stuff as there was before, didn't understand that circus was not needed anymore, we can remove proc-requirements then. Lib Requirements is there to keep the requirements file as is (didn't want to change it until we reach total parity). Just got a heads up by the mongo library maintainer that they are already using thumbor 7 in prod with their library and it's working like a charm.

@gingerlime
Copy link
Contributor

Thank you @heynemann. I really appreciate your help here!

I just had one comment, and I will try to test it with our AWS buckets...

You may say using result storage is cheating, but I'd argue that's the beauty of Thumbor's architecture. It does not depend on the server being incredibly fast (since we need to do things that are CPU intensive like face detection), but it does scale fairly well given the different layers of storage and result storage.

Yes, I completely agree. However it depends what you're trying to measure. Caching / storing results is a very powerful optimization technique. In many scenarios however, maintaining state is undesirable or not possible, and also your system probably needs to be able to scale for a "cold start" scenario, and then the raw performance of the engine is important to understand. The benchmarks I ran so far were looking at this raw performance. This was done in two different contexts: the first one was to understand how best to run thumbor (using threads, docker processes or thumbor processes; the work that @kkopachev did there was incredible and we now know that the thumbor processes are the way to go, and now they come out-of-the-box). The second one I'm working on now is comparing to imagor, which is based on go + libvips. It shows great potential, but of course, nothing is free, and reaching the maturity, stability and feature-richness of thumbor is not easy at all ... not to mention community :) but it's still interesting to see what's out there, and what the possibilities are.

In any case, I guess this part of the discussion is not directly related to this PR, but I still thought it's an interesting conversation to have :)

Let's continue to work in all directions, but of course, I'm keen to get thumbor 7.0 in a stable state with a docker image that is as backwards-compatible as possible with 6.x ...

@gingerlime gingerlime mentioned this pull request Jan 8, 2022
8 tasks
@gingerlime
Copy link
Contributor

merged this into #103

@gingerlime gingerlime closed this Jan 8, 2022
gingerlime added a commit that referenced this pull request May 6, 2023
* updated Dockerfile and docker-entrypoint based on
  #67 (comment)
* moving from Semaphore to Github actions
* using thumbor's built-in multiprocess
* taking changes from #101
* moving docker images to Github (ghcr.io)
* updated requirements with some tc (thumbor community) dependencies that support python 3 / thumbor 7
* added multiarch support (thanks @danquack and @mpdude)
* added option to specify THUMBOR_HOST (thanks @roman-stelmakh-emesa)

TODO

* [x] [circus / multiprocess is broken](#67 (comment))
* [x] move dependencies to `requirements.txt` ?
* [x] add more libs to `requirements.txt` (which?)
* [x] is the conf file backwards compatible?
* [x] ModuleNotFoundError: No module named 'urllib2'
* [x] remotecv image isn't created/pushed? @danquack @mpdude ?
* [ ] caching/performance @danquack @mpdude 
* [ ] testing

---

* refs #67 Thumbor 7.0.0a2

* updated Dockerfile and docker-entrypoint based on
  #67 (comment)
* updated push, so we will push alpha tags on this branch, but not
  overwriting `latest` (and also semaphore CI)

* fixing procs to 1, to bypass circus issue... tests might pass?

* updated semaphore vars

* Revert "updated semaphore vars"

This reverts commit 354a14f.

* kk-multiprocess build

* updating benchmarks

* merging multiproc + 7a2 => 7a5

* fallback values

* allowing custom commands in entrypoint

* fixed command

* empty commit

* Restore OS packages

* Currently working without using tc-aws

* preparing for thumbor 7.0

* removed proc-requirements copy

* I think we still need envtpl for the config

* moving lib-requirements to requirements

* using python 3.9

* cleanup; switching to nginx-proxy/nginx-proxy (github project moved)

* removed build for deprecated nginx-proxy

* apparently, the docker hub image still uses jwilder...

* update cairosvg

* envtpl version to 0.7.2 to fix AttributeError: module 'jinja2' has no attribute 'contextfunction'

* Create GHA for building

* bump to thumbor 7.4.7 (latest)

* moved requirements to txt ; updated community versions to latest with python3 support

* fixed typos

* added bats tests to github actions

* split bats install and run

* nginx-proxy-cache image might be required for testing

* fixes bats tests (#120)

* refactor github action to run sequentially (#121)

* using build script
* based on #116
  from @danquack (Thank you!)
* one job runs sequentially
* builds and loads
* runs tests
* pushes if branch is master or thumbor-7 (for now) using the build
  script

* testing thumbor-7 push

* removed semaphore ; update push for thumbor-7

* explicit branch names

* adding gha caching

* switching to ghcr.io

* login to ghcr.io

* testing push

* trying explicit username/repo name

* Revert "trying explicit username/repo name"

This reverts commit 4f63c79.

* Revert "testing push"

This reverts commit 263740c.

* Revert "Revert "testing push""

This reverts commit b6b95b5.

* fix tags

* removing local tag for push

* change order

* explicit registry name

* another attempt to push to ghcr (using personal access token)

* another attempt with GITHUB_TOKEN

* switching to personal access token

* docker login

* use action

* One more pass at trying to get this to work

* Revert "One more pass at trying to get this to work"

This reverts commit f40ef14.

* Revert "use action"

This reverts commit 39fe365.

* Revert "docker login"

This reverts commit 4a31dc4.

* Remove Multi Arch support from SIMD (#126)

* Disable multi arch support for SIMD images

* updated docs + recipes for GHCR

* adding retry to tests (setup using docker-compose can be unpredictable)

* local tags for testing

* base tag for testing

* explicitly using :test tags

* improve workflow to login to ghcr only when pushing

* showing available images

* better caching?

* adding more info before running tests about images

* switched from imgur (returns 429) to freeimage.host

* fixes tests with new cached data

* updated docs to remove imgur and use freeimage.host

* Overwrite host address 0.0.0.0 with THUMBOR_HOST (#129)

* Overwrite host address 0.0.0.0 with THUMBOR_HOST

---------

Co-authored-by: Roman Stelmakh <47328012+roman-stelmakh-emesa@users.noreply.github.com>

* refs ghcr.io

* typo

* cleanup circus

* syncing remotecv with explicit thumbor version

* tweak build script to add pull and adjust platform for remotecv

* should fix remotecv problem

---------

Co-authored-by: Yoav <yoav@gingerlime.com>
Co-authored-by: Bernardo Heynemann <heynemann@gmail.com>
Co-authored-by: maximka777 <maksim.borovskij@yandex.ru>
Co-authored-by: Daniel Quackenbush <25692880+danquack@users.noreply.github.com>
Co-authored-by: Roman Stelmakh <47328012+roman-stelmakh-emesa@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants