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

Global state on redis not shared between processes #307

Closed
sunderww opened this issue Oct 21, 2020 · 10 comments
Closed

Global state on redis not shared between processes #307

sunderww opened this issue Oct 21, 2020 · 10 comments

Comments

@sunderww
Copy link

sunderww commented Oct 21, 2020

Hi,

I'm trying to serve D-Tale servers from a Django API. The API is responsible for retrieving the correct data, and then calls dtale.show(df).
As stated in the documentation, D-Tale does use a different set of globals in each gunicorn process, so I used dtale.global_state.use_redis_store('/home/jdoe/dtale_data').
I wrote this line in my Django project's wsgi.py file, where I provide the application variable for gunicorn.

The code is correctly called (I can see the redis databases in the provided path), but the global_state is still not shared between the python processes.

I can't figure out why it is not working as intended. Did I call use_redis_store in the wrong place ? Did I need to do something else ? Or is use_redis_store not working properly ?
The documentation is quite light on this subject and I have no idea where else to look.

Thanks in advance.

@aschonfeld
Copy link
Collaborator

Here's a quick script (redis_test.py) I wrote to be used by gunicorn/redis:

import dtale
import dtale.app as dtale_app

from dtale.cli.loaders.csv_loader import loader_func as load_csv
from dtale.views import startup

dtale.global_state.use_redis_store('/Users/andrewschonfeld/dtale_data')

covid = load_csv(
    path='https://raw.githubusercontent.com/nytimes/covid-19-data/master/us-states.csv',
    parse_dates=['date']
)
codes = load_csv(
    path='https://raw.githubusercontent.com/jasonong/List-of-US-States/master/states.csv',
)
codes = codes.set_index('State').to_dict()['Abbreviation']
covid['state_code'] = covid['state'].map(codes)
covid = covid[~covid.state_code.isnull()]

startup("", data=covid, name='covid_data')
app = dtale_app.build_app(reaper_on=False)

if __name__ == '__main__':
    app.run(host='0.0.0.0', port=8080)

I invoked this with the following command gunicorn --bind 0.0.0.0:5000 redis_test:app

So now if you want to use WSGI I think you can simply create a wsgi.py file with:

from redis_test import app

if __name__ == "__main__":
    app.run()

and then invoke gunicorn --bind 0.0.0.0:5000 wsgi:app

Seems to be working correctly for me. What exception or behavior are you seeing that is looking wrong?

BTW, thanks for your submission 🙏

@sunderww
Copy link
Author

sunderww commented Oct 27, 2020

Hi Andrew,

Thanks for your answer.
I tried it and it is indeed working correctly.

Here are more information about what I'm trying to accomplish :

  • Users will hit a route, that will dynamically create a dataframe and launch a new D-Tale server.
@api_view(['POST'])
def create_dtale_server(request, name):
    df = some_function()
    dtale.show(df, host=ACTIVE_HOST, ignore_duplicate=True, name=name)
  • Users are able to list existing D-Tale servers when requesting another route.
@api_view(['GET'])
def get_all_dtale_servers(request):
    curr_data = global_state.get_data()
    instances = []
    for data_id in curr_data.keys():
        data_obj = DtaleData(data_id, build_url(str(ACTIVE_PORT), ACTIVE_HOST))
        metadata = global_state.get_metadata(data_id)
        name = metadata.get("name")
        # convert pandas timestamp to python dateTime
        time = pd.Timestamp(metadata.get("start"), tz=None).to_pydatetime()
        datetime = time.strftime("%Y-%m-%d %H:%M:%S")
        instances.append(
            {
                'id': data_id,
                'name': name,
                'url': data_obj.build_main_url(data_id=data_id),
                'datetime': datetime
            }
        )
    return Response(instances)

Now, the behaviour that looks wrong to me is:

  1. User creates a new server called foo
  2. User calls get_all_dtale_servers() and randomly sees foo or nothing, depending on the worker it hits
  3. User creates a new server called bar
  4. Both D-Tale servers named foo and bar will have the same url (localhost:40000/dtale/main/1) if I called create_dtale_server() from another worker. If I call from the same worker, they will repectively have localhost:40000/dtale/main/1 and localhost:40000/dtale/main/2 (which is what I want).
  5. User calls get_all_dtale_servers() and randomly sees foo, bar, nothing, or both foo and bar (which is what I want), depending on the worker once again.

It is worth noting that when I launch my django app without gunicorn, there is only one worker so everything works perfectly.

I can try to write a minimalist Django app to show you, if you want, but it will take some time. Please tell me if it is needed.

Thanks for your help ! :)

@aschonfeld
Copy link
Collaborator

So I'm not using django, but I was able to get mine working using Flask and the following:

import dtale
import pandas as pd

from dtale.app import build_app, get_instance

from dtale.views import startup
from flask import redirect, jsonify

dtale.global_state.use_redis_store('/Users/andrewschonfeld/dtale_data')

app = build_app(reaper_on=False)

@app.route("/create-df")
def create_df():
    df = pd.DataFrame(dict(a=[1, 2, 3], b=[4, 5, 6]))
    instance = startup(data=df, ignore_duplicate=True)

    return redirect(f"/dtale/main/{instance._data_id}", code=302)

@app.route('/active-instances')
def get_all_dtale_servers():
    instances = []
    for data_id in dtale.global_state.get_data().keys():
        data_obj = dtale.get_instance(data_id)
        metadata = dtale.global_state.get_metadata(data_id)
        name = metadata.get("name")
        # convert pandas timestamp to python dateTime
        time = pd.Timestamp(metadata.get("start"), tz=None).to_pydatetime()
        datetime = time.strftime("%Y-%m-%d %H:%M:%S")
        instances.append(
            {
                'id': data_id,
                'name': name,
                'url': data_obj.main_url(),
                'datetime': datetime
            }
        )
    return jsonify(instances)

The only problem that I have to look into is that when running with gunicorn if I try to specify a route for "/" it will still default to the base root for D-Tale which allows users to upload CSV/TSV, JSON from the web or sample datasets.

Now if you're sticking with Django I did build out a sample app which allows you to still incorporate D-Tale into your app. Here is the source

Let me know if you have any questions. Good luck 🙏

@aschonfeld
Copy link
Collaborator

aschonfeld commented Nov 1, 2020

Ughh, nevermind. I completely forgot to set the workers to a value greater than 1. You're right this seems to be a problem with using redis with gunicorn. Let me see if I can find another solution.

@aschonfeld
Copy link
Collaborator

@sunderww so this was stupid on my part. Looks like redis will re-create the DB as each process connects to it. I was able to prove this out by simply running following:

  • starting one flask process with the code I sent earlier
  • hitting /create-df & /active-instances to validate data is there
  • opening a python terminal and running dtale.global_state.use_redis_store('/Users/andrewschonfeld/dtale_data') then this dtale.global_state.DATA.keys() and it returned no keys
  • I then hit /active-instances again and the keys were gone there too, which led me to my conclusion that connecting processes to redis instances already running will clear them
  • once I hit /create-df again on the flask process & dtale.global_state.DATA.keys() on the python terminal I was seeing things in sync

So from there I wondered "well what if I was able to start up all my flask workers at once then everything should be fine". So I ran the following command:
gunicorn --workers=4 --preload redis_test:app

And from there everything seemed to work as expected. So the key is --preload when starting gunicorn. The only snag I can think of is that if you wanted to preload some data up front when starting gunicorn so it would be readily available to all the flask workers then you might be in trouble. But there has to be some option to allow redis to reuse a pre-existing instance rather than create a new one as processes connect to it.

Let me know if it works for you and I'll work on drafting some documentation

@sunderww
Copy link
Author

sunderww commented Nov 4, 2020

Thanks soooo much for your help, and sorry for the late reply, I didn't have time to explore your solution before today.

I used preload when starting gunicorn and it worked perfectly ! Now everything is working like I wanted and I can use the awesome features of D-Tale :D

if you wanted to preload some data up front when starting gunicorn so it would be readily available to all the flask workers then you might be in trouble

Can you elaborate on that ? As I understand it, I could just write a few lines where I call dtale.global_state.use_redis_store to load some data, wouldn't that work ?

I'll work on drafting some documentation

Can I offer some help on that ?

@aschonfeld
Copy link
Collaborator

Oh that's awesome! Glad to hear it. Here's my documentation so far: https://github.com/man-group/dtale/blob/bugfixes_20201102/docs/GUNICORN_REDIS.md

So feel free to just upload any updates to that file here or just wait until I try releasing this stuff this weekend and you can create a PR to the repo for any changes you want to make. I might also try adding an example of what you're asking about for pre-loading a dataframe up front to be shared.

@aschonfeld
Copy link
Collaborator

@sunderww just wanted to give you a heads up that I merged that documentation into master and I fixed that issue with overriding routes on gunicorn in the release I just put out v1.21.0.

Feel free to close this issue if you think it's complete, thanks

@sunderww
Copy link
Author

sunderww commented Nov 10, 2020

@aschonfeld The documentation on D-Tale with flask and D-Tale with Django is very clear, nice work !

I might also try adding an example of what you're asking about for pre-loading a dataframe up front to be shared.

I actually don't need to do that. I just wanted to understand more clearly, so don't trouble yourself with this if that wasn't on your priorities 😃

Feel free to close this issue if you think it's complete, thanks

Yes everything works perfectly now, so I'll close the issue. Thanks again for your help :)

@aschonfeld
Copy link
Collaborator

aschonfeld commented Nov 12, 2020

So here's an example of some code that does some preloading of data on startup (bear in mind this won't work with the current version because I found a bug):

import dtale
import pandas as pd

from dtale.app import build_app

from dtale.views import startup
from flask import redirect, jsonify


app = build_app(reaper_on=False)

dtale.global_state.use_redis_store('/Users/andrewschonfeld/dtale_data')

if '1' not in dtale.global_state.get_data():
    df = pd.DataFrame([1, 2, 3, 4, 5])
    startup(data=df, data_id=1)


@app.route("/create-df")
def create_df():
    df = pd.DataFrame(dict(a=[1, 2, 3], b=[4, 5, 6]))
    instance = startup(data=df, ignore_duplicate=True)

    return redirect(f"/dtale/main/{instance._data_id}", code=302)


@app.route("/")
@app.route("/active-instances")
def get_all_dtale_servers():
    instances = []
    for data_id in dtale.global_state.get_data().keys():
        data_obj = dtale.get_instance(data_id)
        metadata = dtale.global_state.get_metadata(data_id)
        name = metadata.get("name")
        # convert pandas timestamp to python dateTime
        time = pd.Timestamp(metadata.get("start"), tz=None).to_pydatetime()
        datetime = time.strftime("%Y-%m-%d %H:%M:%S")
        instances.append(
            {
                'id': data_id,
                'name': name,
                'url': data_obj.main_url(),
                'datetime': datetime
            }
        )
    return jsonify(instances)


if __name__ == '__main__':
    app.run(host="0.0.0.0", port=8080)

Make a note of these lines:

if '1' not in dtale.global_state.get_data():
    df = pd.DataFrame([1, 2, 3, 4, 5])
    startup(data=df, data_id=1)

Now if you start that up using gunicorn when you bring it up in a browser you should see an instance already available:

image

And then you can see the data by jumping directly to http://http://127.0.0.1:8000/dtale:

image

Now this is using meaningless data, but if you wanted some sort of reference data available when the application starts this is how you'd do it.

The good thing is that it apparently works 😄 (well of course after I release that bugfix)

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

No branches or pull requests

2 participants