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

Celery experiment - Take 2 #727

Merged
merged 1 commit into from
Oct 17, 2017
Merged

Celery experiment - Take 2 #727

merged 1 commit into from
Oct 17, 2017

Conversation

r-darwish
Copy link
Collaborator

Same as before, but as suggested here we now run Celery in Heroku without the -D flag, which means the process will not detach itself from its creator. Maybe now we can see some informative logs when something goes wrong.

@danielhers danielhers temporarily deployed to anyway-unstable-pr-727 September 12, 2017 06:22 Inactive
@danielhers
Copy link
Collaborator

Thanks. Still getting timeouts, and I'm not seeing anything relevant in the logs:

2017-09-12T06:40:07.752181+00:00 heroku[router]: at=error code=H12 desc="Request
 timeout" method=GET path="/clusters?ne_lat=32.097011473183386&ne_lng=34.8206795
8798825&sw_lat=32.064139992645615&sw_lng=34.761456412939424&zoom=15&thin_markers
=true&start_date=1410566400&end_date=1505174400&show_fatal=1&show_severe=1&show_
light=1&approx=1&accurate=1&show_markers=1&show_discussions=1&show_urban=3&show_
intersection=3&show_lane=3&show_day=7&show_holiday=0&show_time=24&start_time=25&
end_time=25&weather=0&road=0&separation=0&surface=0&acctype=0&controlmeasure=0&d
istrict=0&case_type=0" host=anyway-unstable-pr-727.herokuapp.com request_id=9973
2597-f77b-4685-ac85-ae2a767feaac fwd="212.143.240.235" dyno=web.1 connect=0ms se
rvice=30000ms status=503 bytes=0 protocol=https
2017-09-12T06:40:08.721002+00:00 app[web.1]: 2017-09-12 06:40:08 [4] [CRITICAL]
WORKER TIMEOUT (pid:49)
2017-09-12T06:40:08.712534+00:00 app[web.1]: 2017-09-12 06:40:08 [4] [CRITICAL]
WORKER TIMEOUT (pid:49)
2017-09-12T06:40:08.723552+00:00 app[web.1]: 2017-09-12 06:40:08 [72] [INFO] Boo
ting worker with pid: 72
2017-09-12T06:40:09.287733+00:00 app[web.1]: DEBUG:passlib.registry:registered '
des_crypt' handler: <class 'passlib.handlers.des_crypt.des_crypt'>
2017-09-12T06:40:09.290317+00:00 app[web.1]: DEBUG:passlib.registry:registered '
pbkdf2_sha512' handler: <class 'passlib.handlers.pbkdf2.pbkdf2_sha512'>
2017-09-12T06:40:09.292419+00:00 app[web.1]: DEBUG:passlib.registry:registered '
sha256_crypt' handler: <class 'passlib.handlers.sha2_crypt.sha256_crypt'>
2017-09-12T06:40:09.283600+00:00 app[web.1]: DEBUG:passlib.registry:registered '
bcrypt' handler: <class 'passlib.handlers.bcrypt.bcrypt'>
2017-09-12T06:40:09.293042+00:00 app[web.1]: DEBUG:passlib.registry:registered '
plaintext' handler: <class 'passlib.handlers.misc.plaintext'>
2017-09-12T06:40:09.290188+00:00 app[web.1]: DEBUG:passlib.registry:registered '
pbkdf2_sha256' handler: <class 'passlib.handlers.pbkdf2.pbkdf2_sha256'>
2017-09-12T06:40:09.292509+00:00 app[web.1]: DEBUG:passlib.registry:registered '
sha512_crypt' handler: <class 'passlib.handlers.sha2_crypt.sha512_crypt'>

@r-darwish
Copy link
Collaborator Author

This is weird. Are there no logs at all from worker?

@danielhers
Copy link
Collaborator

None that I can see. I'm running heroku logs --app anyway-unstable-pr-727 and only seeing logs from app and heroku...

@r-darwish
Copy link
Collaborator Author

I'll try to play with Heroku later today

@simonaho
Copy link
Collaborator

I talked with @uda yesterday and he promised to complete the new integration server this week, if you want to do the integration on the same type of server as the deployment server.

@danielhers danielhers temporarily deployed to anyway-unstable-pr-727 September 12, 2017 17:29 Inactive
@danielhers danielhers temporarily deployed to anyway-unstable-pr-727 September 12, 2017 17:37 Inactive
@r-darwish
Copy link
Collaborator Author

I turned on worker in the resource screen at the GUI and now I get logs from the worker. However, I don't think we have Rabbit MQ as it seems to be a beta plugin for Heroku which is available for invites only. Although RabbitMQ is the most popular, Celery supports other backends such as Redis. I can make the backend configurable so that we'll use Redis in Heroku and RabbitMQ in production.

@danielhers danielhers temporarily deployed to anyway-unstable-pr-727 September 12, 2017 19:29 Inactive
@danielhers danielhers temporarily deployed to anyway-unstable-pr-727 September 12, 2017 19:35 Inactive
@r-darwish
Copy link
Collaborator Author

@danielhers works now

@danielhers
Copy link
Collaborator

Looks good. So is it RabbitMQ on Travis CI but Redis on Heroku?
@uda Need to set it up on our new server too.

@r-darwish
Copy link
Collaborator Author

Yup. Celery backends should be transparent so we shouldn't change anything in the code when we switch between them. All we need to do is set up 2 environment variables. Both Redis and RabbitMQ are find but I think RabbitMQ is considered better so I think that should be the choice for the production server.

@uda / @danielhers, can you tell me which Linux distribution and version do we use in production?

Also, I'm not sure how easy it is to set up Celery in Windows so if there's anyone who develop on Windows I'll be happy to know of setting up Celery is doable.

@danielhers
Copy link
Collaborator

@r-darwish it's Ubuntu 16.04.

@r-darwish
Copy link
Collaborator Author

Good. That means we can also get Python 3.5. I'll update the instructions for developers later today, but in terms of code I think it's ready.

@simonaho
Copy link
Collaborator

Is there a way to disable this for local environment? So we can run it on Windows?
This is since local environment does not need the performance improvement.

@simonaho
Copy link
Collaborator

I prefer to push this after we have a solution for all developers

@r-darwish
Copy link
Collaborator Author

Yeah, we can do that. I think that it's best that we first know if it's possible to get Celery in Windows and then decide whether we want a mode for disabling it. Is there any developer who uses Windows that can check?

@danielhers danielhers temporarily deployed to anyway-unstable-pr-727 September 13, 2017 18:16 Inactive
@r-darwish
Copy link
Collaborator Author

r-darwish commented Sep 13, 2017

I looked into the documentation and it seems that Celery isn't officially supported on Windows. So, we have two choices:

  1. Assuming that Celery can be still installed and imported on Windows, we can write an abstraction layer that will allow us to disable Celery when needed. This layer is easy to implement now, but if we decide in the future that we want to use more advanced features of Celery, then our life will become more difficult.

  2. Decide that we don't support running Anyway on Windows. This absolutely doesn't mean that Anyway can't be developed on a Windows machine (Actually, the computer I develop on runs Windows 10). There are multiple options for a Windows user to run Anyway: Docker, Vagrant, Linux Subsystem for Windows 10 and running your own VM. I use that latter. The Linux subsystem is an official part of Windows made by Microsoft.

@simonaho, that's something for you to decide as the project manager, but I personally recommend going with 2, as there are multiple options for Windows users. Plus, our current installation instructions for Windows are marked as "experimental". I don't know if they even work without Celery.

@aviklai
Copy link
Collaborator

aviklai commented Sep 13, 2017

@r-darwish What about using the 3.x branch of celery?

according to the following post:
https://stackoverflow.com/questions/37255548/how-to-run-celery-on-windows

Version 3.1.25 does support windows.

@r-darwish
Copy link
Collaborator Author

That means we won't be able to upgrade Celery. I personally don't like the idea of sticking with an outdated version, but that's also an option.

@simonaho
Copy link
Collaborator

Do you have a good experience with appVeyor?

@r-darwish
Copy link
Collaborator Author

I have no experience :)

I think I'll integrate it in the Anyway SDK first and see how it goes.

@simonaho
Copy link
Collaborator

simonaho commented Sep 14, 2017

My experience with Linux VM on Windows wasn't a good one, and Windows VM on MAC OS also has its issues, so I'm a bit concerned about VMs. That's not a methodological conclusion. But maybe a quick google search can give us some indication about this before you invest time in it.

@danielhers danielhers temporarily deployed to anyway-unstable-pr-727 September 15, 2017 14:24 Inactive
@r-darwish
Copy link
Collaborator Author

@simonaho I rebased it on the top of the dev branch and it seems to pass AppVeyor checking, so it works on Windows.

@simonaho
Copy link
Collaborator

Sound like great news!
(I hope there will be no surprises on real Windows machine, although I assume there shouldn't be)
Indeed quite responsive clusters on the server.
Do we want to wait for @uda with the integration server from the same type as the production server, before merge? Hope its going to be ready soon as promised.
Thank you @r-darwish for this great contribution

@r-darwish
Copy link
Collaborator Author

That's something for you and @uda to decide. Performing the transition on an identical integration server will probably mean less chance for unexpected things to happen when deploying to production.

@simonaho
Copy link
Collaborator

Yes, so I thought it would be be better to wait with the merge until the integration, not to stop the other changes, if required in production.

@aviklai
Copy link
Collaborator

aviklai commented Sep 18, 2017

The clusters xhr request is stuck on the window dev environment.
I tried to add the ANYWAY_DISABLE_CELERY env variable and it didn't work.

@r-darwish
Copy link
Collaborator Author

Can you put a PDB break above this if and check what happens?

@aviklai
Copy link
Collaborator

aviklai commented Sep 18, 2017

It enters inside the if statement.

@r-darwish
Copy link
Collaborator Author

Okay. Can you step into this location with a debugger and see what happens?

@aviklai
Copy link
Collaborator

aviklai commented Sep 19, 2017

Strange - the problem is in the frontend. I see that the xhr data is sent from the server and I get it on the client but the callback function "markersFetched" is never fired and the browser is stuck.

@r-darwish
Copy link
Collaborator Author

That's weird. Which browser are you using and which extensions do you have installed? Also, does the dev branch works for you? Because the last time I touched that area was in 11be8da and b407f67 which were already merged to dev

@aviklai
Copy link
Collaborator

aviklai commented Sep 20, 2017

I tries on chrome and firefox - the same issue occurs on both of the browsers and on firefox I don't have any extensions installed. The dev branch works - it happens only in the celery branch.

@danielhers danielhers temporarily deployed to anyway-unstable-pr-727 September 20, 2017 09:34 Inactive
@r-darwish
Copy link
Collaborator Author

I had a bug in which I returned the wrong type when Celery was disabled. I returned a list of lists instead of a single list. Apparently, the unit test of the cluster view just checks that it got any type of result, so I fixed it as well to prevent such mistakes in the future. I also rebased this on the top of develop.

@aviklai thanks for reporting. It should work now.

@aviklai
Copy link
Collaborator

aviklai commented Sep 20, 2017

@r-darwish great :) now it works.

@r-darwish
Copy link
Collaborator Author

As discussed with @uda, I am merging this now to dev. I also updated the Docker compose files to run Celery.

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.

4 participants