Commit Graph

39510 Commits

Author SHA1 Message Date
Steve Howell 1951d75796 performance: Avoid select_related("realm").
We also move this query up in the function
for some future refactorings.
2020-10-18 14:27:31 -07:00
Steve Howell 2f8ba383ef tests: Test overhead for creating new users. 2020-10-18 14:27:31 -07:00
YashRE42 51f82ad839 right_sidebar: Rename .css file to right_sidebar.css.
This changes the file name to existing conventions, it does not,
however, change the actual `.right_sidebar` tag used in the
app.
2020-10-18 14:09:47 -07:00
YashRE42 bd72ba76f3 left-sidebar: Rename .css file to left_sidebar.css.
This changes the file name to existing conventions, it does not,
however, change the actual `.left_sidebar` tag used in the
app.
2020-10-18 14:09:47 -07:00
YashRE42 46a74d3e08 informational-overlays: Rename .css file to informational_overlays.
This changes the file name to existing conventions, it does not,
however, change the actual `.informational-overlay` tag used in the
app.
2020-10-18 14:09:47 -07:00
Mateusz Mandera 716df658fa queue_processors: Don't run test queues with run-dev.py. 2020-10-18 14:07:31 -07:00
Mateusz Mandera 269faa342a test-queue-worker-reload: Fix expected worker count.
We didn't bump this when adding some additional test queues, that run
with run-dev.py as of this commit.
2020-10-18 14:07:31 -07:00
Aman Agrawal 1e5873df9e text-error-css: Use same color as used by bootstrap for text-error. 2020-10-18 14:06:20 -07:00
Aman Agrawal 1849ca41d2 login: Top right align error message.
Fixes the alignment issues and this better indicates the field to
which error message belongs to.
2020-10-18 14:06:20 -07:00
Aman Agrawal 8297121050 Revert "login: Fix alignment of error message in login form."
This reverts commit 6fe82620df.

Reverted since this misaligns other forms which use the same
class.
2020-10-18 14:06:20 -07:00
YashRE42 178e0b7d47 navbar: Use location.href instead of location.replace.
As per https://stackoverflow.com/questions/1865837/ location.href
should be preferred to location.replace in some places due to the
fact that location.replace violates browser history and breaks back key.
2020-10-18 14:05:45 -07:00
Aman Agrawal bdc4721aee BaseDocumentationSpider: Don't crawl webapp. 2020-10-16 16:26:29 -07:00
Aman Agrawal 72b25553b2 BaseDocumentationSpider: Print correct link on error. 2020-10-16 16:23:33 -07:00
Alex Vandiver 7a1132d605 puppet: Switch golang and smokescreen to use /srv.
/srv and /opt have very similar usages; but we should be internally
consistent.

Move these two (the only usages of /opt) to match the rest in /srv.
2020-10-16 13:00:06 -07:00
Steve Howell 57efe9d81a performance: Streamline list_to_streams.
We take advantage of stream.recipient to simplify
the query's where clause and avoid the need
for select_related("recipient").
2020-10-16 12:58:11 -07:00
Steve Howell e1bcf6124f refactor: Remove recipient from access_stream_by_name. 2020-10-16 12:58:11 -07:00
Steve Howell a51b483f1a performance: Remove recipient from access_stream_by_id.
The Recipient table is now kind of useless for
stream-related operations, since we have
recipient_id on Stream now.
2020-10-16 12:58:11 -07:00
Steve Howell 31622feb87 refactor: Only return sub from access_stream_common.
Let the callers access stream.recipient as needed.
It costs the same, and some of the callers can
actually stop caring about the actual Recipient
object.
2020-10-16 12:58:11 -07:00
Steve Howell bfd6e2b1fd refactor: Use recipient_id to get topic history. 2020-10-16 12:58:11 -07:00
Steve Howell 3685fcc701 refactor: Remove recipient arg for do_mute_topic. 2020-10-16 12:58:11 -07:00
Steve Howell 65dbee4837 minor: Ask for recipient_id, not recipient. 2020-10-16 12:58:11 -07:00
Steve Howell 378062cc83 performance: Avoid call to access_stream_by_id.
We already trust ids that are put on our queue
for deferred work. For example, see the code for
"mark_stream_messages_as_read_for_everyone"

We now pass stream_recipient_id when we queue
up work for do_mark_stream_messages_as_read.

This generally saves about 3 queries per
user when we unsubscribe them from a stream.
2020-10-16 12:58:11 -07:00
Steve Howell 2256d72015 minor: Add comment to subscriber test. 2020-10-16 12:58:11 -07:00
Steve Howell 31eb97ddde performance: Fix do_mark_stream_messages_as_read.
This function no longer asks for data that it
doesn't need.
2020-10-16 12:58:11 -07:00
Steve Howell 6d1f9de7d3 performance: Use SubInfo when removing subscribers.
We get two speedups:

    * The query to get existing subscribers only
      gets the two fields we need.  We no longer
      need all the overhead of user_profile
      and recipient data being returned in the
      query.

    * We avoid Django making extra hops to the
      database to get user info.
2020-10-16 12:58:11 -07:00
Steve Howell 73982f6cc9 refactor: Move SubInfo to stream_subscription.py. 2020-10-16 12:58:11 -07:00
Anders Kaseorg 16aa48d9b2 configure-rabbitmq: Wait for RabbitMQ to start up.
Fixes an occasional failure in ‘vagrant up --provision’.

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-10-15 17:01:00 -07:00
Anders Kaseorg f16aa8f264 configure-rabbitmq: Put the command and flags in one array.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-10-15 17:01:00 -07:00
Alex Vandiver 1fa4ef0271 upgrade-postgres: Catch failed pg_upgradecluster exit code.
Because the command is part of a pipe sequence, the exitcode defaults
to the last in the sequence, which is not the most important one here.

Set pipefail, which sets the exit status to the exit code of the last
program in the sequence to exit non-zero, or 0 if all succeeded.  This
prevents the upgrade from barreling onward and setting
`postgres.version` improperly if the database upgrade step failed.
2020-10-15 15:21:30 -07:00
Alex Vandiver 78b92a51cc puppet: Allow access to smokescreen port via iptables. 2020-10-15 15:18:35 -07:00
Alex Vandiver 0d5356969e puppet: Reformat ipv4 iptables rules comments. 2020-10-15 15:18:35 -07:00
Alex Vandiver fffea9612b puppet: Add an outgoing HTTP/HTTPS proxy server.
Use https://github.com/stripe/smokescreen to provide a server for an
outgoing proxy, run under supervisor.  This will allow centralized
blocking of internal metadata IPs, localhost, and so forth, as well as
providing default request timeouts (10s by default).
2020-10-15 15:18:35 -07:00
Anders Kaseorg 6099612a15 lint: Add shfmt as a linter.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-10-15 15:16:00 -07:00
Anders Kaseorg dfaea9df65 shfmt: Reformat shell scripts with shfmt.
https://github.com/mvdan/sh

Signed-off-by: Anders Kaseorg <anders@zulip.com>
2020-10-15 15:16:00 -07:00
Tim Abbott caa939d2d5 actions: Use transaction.atomic properly when removing subscriptions.
Previously, the transaction.atomic() was not properly scoped to ensure
that RealmAuditLog entries were created in the same transaction,
making it possible for state changes to not be properly recorded in
RealmAuditLog.
2020-10-15 15:12:05 -07:00
Steve Howell 0b91526f28 events: Remove "occupied" semantics for "streams".
When apps like mobile register for "streams", we
will now just use active streams as our baseline,
rather than "occupied" streams.

This means we will send a stream that is active,
even if it happens to have zero occupants.  It's
actually pretty rare that a stream has zero occupants,
and it's not exactly clear that we want to exclude
a non-occupied but otherwise active stream from
our list of streams.

It also happens to be fairly expensive to compute
whether a stream is occupied.

This change only affects API clients (including
possibly our mobile app).  The main webapp never
used the data from this codepath.
2020-10-15 15:12:01 -07:00
Steve Howell b4346d0276 performance: Extract subscribers/peers in bulk.
We replace get_peer_user_ids_for_stream_change
with two bulk functions to get peers and/or
subscribers.

Note that we have three codepaths that care about
peers:

    subscribing existing users:
        we need to tell peers about new subscribers
        we need to tell subscribed user about old subscribers

    unsubscribing existing users:
        we only need to tell peers who unsubscribed

    subscribing new user:
        we only need to tell peers about the new user
        (right now we generate send_event
        calls to tell the new user about existing
        subscribers, but this is a waste
        of effort that we will fix soon)

The two bulk functions are this:

    bulk_get_subscriber_peer_info
    bulk_get_peers

They have some overlap in the implementation,
but there are some nuanced differences that are
described in the comments.

Looking up peers/subscribers in bulk leads to some
nice optimizations.

We will save some memchached traffic if you are
subscribing to multiple public streams.

We will save a query in the remove-subscriber
case if you are only dealing with private streams.
2020-10-15 15:12:01 -07:00
Steve Howell 94e41c71f9 refactor: Use set of ids for altered users. 2020-10-15 15:12:01 -07:00
Steve Howell b894597fa3 refactor: Use sets of stream_ids for helper args. 2020-10-15 15:12:01 -07:00
Steve Howell 3889554977 refactor: Extract send_peer_remove_events. 2020-10-15 15:12:01 -07:00
Steve Howell c73f84f275 tests: Improve tests for unsubscribing multiple users.
Note that the tests now reflect that we have O(N)
behavior for multiple users.
2020-10-15 15:12:01 -07:00
Steve Howell f86823f82f tests: Add cache_tries_captured helper. 2020-10-15 15:12:01 -07:00
Steve Howell ce70d08cbf test_helpers: Use mock.patch.multiple. 2020-10-15 15:12:01 -07:00
areebbeigh b217cce80f docs: Fix various typos.
This commit fixes typos in:
tutorials/new-feature-tutorial.md
subsystems/sending-messages.md
2020-10-15 14:39:43 -07:00
Alex Vandiver 0d5760d59f install-aws-server: Force non-interactive dist-upgrade.
Installing an updated linux kernel package, as can happen during the
`apt dist-upgrade` done by the installer, can cause grub to pop up a
prompt to update its configuration file.  In an unattended headless
configuration, this will stop the installation.

Explicitly configure apt to be non-interactive, and prefer the newest
configuration, during the install.
2020-10-15 14:39:20 -07:00
Anders Kaseorg 449b16be59 dependencies: Use patched source-map with a performance fix.
https://github.com/mozilla/source-map/issues/370

This makes Webpack about 9% faster.  Before:

$ multitime -n9 -s0 node_modules/.bin/webpack --config-name=frontend --display=errors-only -p
===> multitime results
1: node_modules/.bin/webpack --config-name=frontend --display=errors-only -p
            Mean        Std.Dev.    Min         Median      Max
real        18.243      0.107       18.090      18.236      18.443
user        27.913      0.188       27.714      27.843      28.251
sys         2.028       0.043       1.933       2.039       2.074

After:

$ multitime -n9 -s0 node_modules/.bin/webpack --config-name=frontend --display=errors-only -p
===> multitime results
1: node_modules/.bin/webpack --config-name=frontend --display=errors-only -p
            Mean        Std.Dev.    Min         Median      Max
real        16.686      0.085       16.542      16.684      16.885
user        25.965      0.167       25.559      26.022      26.163
sys         1.965       0.064       1.807       1.998       2.010

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
2020-10-15 14:38:41 -07:00
Tim Abbott bf66e9c4ab actions: Add transaction.atomic to bulk_add_subs_to_db_with_logging.
This will ensure that we always fully execute the database part of
modifying subscription objects.  In particular, this should prevent
invariant failures like #16347 where Subscription objects were created
without corresponding RealmAuditLog entries.

Fixes #16347.
2020-10-14 11:06:00 -07:00
Steve Howell 5728149e94 performance: Streamline query to add subscribers.
We don't need the select_related('user_profile')
optimization any more, because we just keep
track of user info in our own data structures.

In this codepath we are never actually modifying
users; we just occasionally need their ids or
emails.

This can be a pretty substantive improvement if
you are adding a bunch of users to a stream
who each have a bunch of their own subscriptions.

We could also limit the number of full rows in this
query by adding an extra hop to the DB just to
get colors (using values_list), and then only get
full sub info for the streams that we're adding, rather
than getting every single subscription, in full, for each user.

Apart from finding what colors the user has already
used, the only other reason we need all the columns
in Subscription here is to handle streams that
need to be reactivated.  Otherwise we could do
only("id", "active", "recipient_id", "user_profile_id")
or similar.  Fortunately, Subscription isn't
an overly wide table; it's mostly bool fields.

But by far the biggest thing to avoid is bringing
in all the extra user_profile data.

We have pretty good coverage on query counts here,
so I think this fix is pretty low risk.
2020-10-14 11:03:07 -07:00
Steve Howell 116a441bc5 refactor: Introduce SubInfo class.
This class removes a lot of the annoying tuples
we were passing around.

Also, by including the user everywhere, which
is easily available to us when we make instances
of SubInfo, it sets the stage to remove
select_related('user_profile').
2020-10-14 10:53:10 -07:00
Steve Howell febef45e38 minor: Add comments to do_get_streams. 2020-10-14 10:53:10 -07:00