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

Speedups for admin interface #56

Closed
wants to merge 3 commits into from

Conversation

Sovetnikov
Copy link
Contributor

  1. Store actor and queue name in model fields
  2. Actor name and queue name filters work directly by model fields
  3. updated_at models field db_index=True

Just having very poor performance without this changes.

@jcass77
Copy link
Contributor

jcass77 commented Nov 4, 2019

At the moment, messages are just bytes that form part of the Task model. Parsing out queue_name and actor_name from this byte stream as new fields on the Task model duplicates the data and probably breaks the neat encapsulation that we currently enjoy a little.

@Bogdanp, is there a reason why messages are retained in encoded format as part of a BinaryField? If not then creating a foreign key to a fully parsed and indexed new Message model feels like a cleaner approach?

@Sovetnikov
Copy link
Contributor Author

@jcass77 actor name and queue name stored in model just speeds up the search in these fields. We have more than 100 000 messages in two weeks and search becomes unusable.
Binary message representation have lots of information that not in model fields, like traceback (I have doubts whether this works, will check this out on failed messages), eta, arguments, pipelines and callbacks data, that very handy in admin interface.
Storing all message attributes in model fields too complex and needs regular maintenance if changes will be made in dramatiq, so full message storage is fine here.

Storage of tasks state and history is temporary by it's nature i think, and performance is more important than normalization and storage size.

@Bogdanp
Copy link
Owner

Bogdanp commented Nov 4, 2019

Thanks! This looks good and I'll take a deeper look at it next weekend. I am fine with denormalizing part of the message if it helps in use cases like these, though I don't know if I'd really recommend using the admin middleware in a performance-sensitive environment in general.

@Bogdanp
Copy link
Owner

Bogdanp commented Nov 30, 2019

Thanks! I've rebased and merged your changes. I'll cut a release next weekend. Sorry this took so long, but I've been extremely busy recently.

@Bogdanp Bogdanp closed this Nov 30, 2019
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.

3 participants