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

Make padwoman fast again! (And several other things) #13

Open
wants to merge 16 commits into
base: development
Choose a base branch
from

Conversation

jomority
Copy link
Contributor

@jomority jomority commented Jan 7, 2022

I've used the last days to hack a bit on padwoman. This pull request represents the result of that hacking.
If you want, I can split it up into more smaller pull requests.
A quick overview of the changes (the commit messages and diffs should explain more):

The bold changes are the ones that will make padwoman fast again. Maybe we could bump the version number?

From now on, we will test the changes in production if you want to wait for a bit before pulling.

According to the etherpad API documentation sessions should be deleted
after logout: https://etherpad.org/doc/v1.8.4/#index_session

We don't do that. So at least clean them up periodically.

Even if we would, we'd have to cleanup periodically because:
* not everyone logs out
* if the redis cache gets cleared we don't have any knowledge of the
  sessions anymore
In preparation for the next commit, which implements session cleanup on
logout, we need the ability to map the etherpad sessions to a padwoman
session and not just the user.
The reason for this is that not all sessions are logged out of etherpad
at once. So a logout on one pc does not log out another session of the
same user.
The keys command should only be used for debugging:
https://redis.io/commands/keys
This setting specifies the column and order by which a padgroup will be
sorted by default.
The default is the old behaviour.
In preparation for the next commit series
This commit is part of a series which goal is to minimize the loading
time of the padlist.

The main reason for slowness was that if the public status or lastEdit
timestamp where not cached in redis, padwoman would crawl them for the
whole pad group from the slow etherpad API before replying with the
page. That resulted unacceptable minute-long loading times.

This commit series will make the page return instantly, omitting the
data not in the redis cache. The remaining data will be fetched from the
etherpad API asynchronously and will be available in subsequent
requests. With JavaScript it is also possible to lazily load the data
without refreshing the page.

This commit removes the periodically scheduled updateTimestamps job. It
will not be necessary anymore.
This commit is part of a series which goal is to minimize the loading
time of the padlist.
See 9386132 for more explanation.

This commit reworks etherpad_cached_api.getPadlist so that it can return
very fast without talking to the etherpad API. If the necessary data
(public, lastEdit) is not in the redis cache, "Unavailable" is used
instead. In the padlist this is represented by a loading symbol.

If data needs to be fetched, a new thread is started that does this. A
mutex is used, so that only one thread fetches data.

If the padlist is sorted by a column with "Unavailable" data, they are
sorted as most recent (first in descending order). This is probably
true, because of the cache time heuristic. As a side effect of the
implementation, it is now possible to sort by the public status.
This commit is part of a series which goal is to minimize the loading
time of the padlist.
See 9386132 for more explanation.

This commit adds a microapi endpoint for getting the padlist. It is
synchronous and queries all missing data from the etherpad API. The
endpoint is used by the client via JavaScript to reload the page when
all data is fetched.
This commit is part of a series which goal is to minimize the loading
time of the padlist.
See 9386132 for more explanation.

This commit properly implements the client side of the lazy loading. The
data from the microapi reply is inserted into the padlist via
JavaScript. The page does not need to be reloaded.
@jomority
Copy link
Contributor Author

I force pushed two small bugfixes which we noticed today.

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.

1 participant