We used 'savepoint=False' in #31169 which was prior to our discussion
in CZO to use 'durable=True' instead.
This commit makes changes to use 'durable=True' in the outermost
transaction.atomic block.
This commit removes the 'prev_rendered_content_version'
field from:
* the 'edit_history' object within message objects in the
API response of `GET /messages`, `GET /messages/{message_id}`
and `POST /zulip-outgoing-webhook`.
* the 'update_message' event type
as it is an internal server implementation detail not used
by any client.
Note: The field is still stored in the 'edit_history' column
of the 'Message' table as it will be helpful when making
major changes to the markup rendering process.
Thumbnails are usually enqueued in the worker when the image is
uploaded. However, for images which were uploaded before the
existence of the thumbnailing worker, and whose metadata was
backfilled (see previous commit) this leaves a permanent spinner,
since nothing triggers the thumbnail worker for them.
Enqueue a thumbnail worker for every spinner which we render into
Markdown. This ensures that _something_ is attempting to resolve the
spinner which the user sees. In the case of freshly-uploaded images
which are still in the queue, this results in a duplicate entry in the
thumbnailing queue -- this is harmless, since the worker determines
that all of the thumbnails we need have already been generated, and it
does no further work. However, in the case of historical uploads, it
properly kicks off the thumbnailing process and results in a
subsequent message update to include the freshly-generated thumbnail.
While specifically useful for backfilled uploads, this is also
generally a good safety step for a good user experience, as it also
prevents dropped events in the queue from unknown causes from leaving
perpetual spinners in the message feed.
Because `get_user_upload_previews` is potentially called twice for
every message with spinners (see 6f20c15ae9), we add an additional
flag to `get_user_upload_previews` to suppress a _second_ event from
being enqueued for every spinner generated.
Since each loop may add more than one file to the `storage_paths`
list, this may result in more than 1000 files being sent to
delete_message_attachments. Since the S3 backend only supports 1000
elements being deleted at once, we must partition the list into chunks
which are no more than 1000 elements long.
There are a few places where we want to set the max invites for a
realm to the default for a realm's plan type, so this creates a
helper function that can be used consistently to get that default
value.
Adds some validation for changing the realm's max invites via the
support view so that it is not set below the default max for the
realm's plan type, and so that if it's currently set to the default
max it's not reset to that same value.
Earlier, we were using 'send_event' in 'do_change_is_billing_admin'
which can lead to a situation, if any db operation is added after
the 'send_event' in future, where we enqueue events but the action
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in
'do_update_outgoing_webhook_service' which can lead to a
situation, if any db operation is added after the 'send_event'
in future, where we enqueue events but the action function fails
at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in
'do_set_push_notifications_enabled_end_timestamp' which
can lead to a situation, if any db operation is added after the
'send_event' in future, where we enqueue events but the action
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in 'do_set_realm_stream' which
can lead to a situation, if any db operation is added after the
'send_event' in future, where we enqueue events but the action
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in
'do_set_realm_authentication_methods' which can lead to a situation
where we enqueue events but there's an error at a later stage in
the codepath using this function.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in do_set_realm_user_default_setting
which can lead to a situation, if any db operation is added after
the 'send_event' in future, where we enqueue events but the action
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in 'do_regenerate_api_key'
which can lead to a situation, if any db operation is added after
the 'send_event' in future, where we enqueue events but the action
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in 'do_change_full_name'
which can lead to a situation, if any db operation is added after
the 'send_event' in future, where we enqueue events but the action
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
This commit updates the db transaction to be durable for
do_update_user_custom_profile_data_if_changed to avoid
addition of any outer atomic block.
While adding any outer atomic block this will raise a runtime error
and we can replace the durable argument with 'savepoint=False'
otherwise we'll have to manually track down the action functions
getting called in that outer atomic block and set the savepoint=False
otherwise it'll lead to creation of savepoints which we don't want.
We can't set savepoint=False before hand to the outermost action
function because it leads to rollback of transaction in tests when
an error is raised in action function.
Earlier, we were using 'send_event' in
check_remove_custom_profile_field_value which can lead to a
situation where we enqueue events but the function fails at a
later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
This commit make changes in code to include can_manage_group
field to UserGroup objects passed with response of various endpoints
including "/register" endpoint and also in the group object
send with user group creation event.
Earlier, we were using 'send_event' in
do_change_stream_message_retention_days which can lead to a situation
where we enqueue events but the function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in do_change_stream_description
which can lead to a situation where we enqueue events but the
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in do_change_stream_post_policy
which can lead to a situation where we enqueue events but the
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
This commit adds a transaction.atomic decorator to the
'do_change_subscription_property' function to make
the db operations in the action function atomic.
Also, send_event is changed to send_event_on_commit.
Earlier, we were using 'send_event' in 'do_unmute_user'
which can lead to a situation where we enqueue events but the
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in 'do_mute_user' which
can lead to a situation where we enqueue events but the
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in do_update_message_flags
which can lead to a situation where we enqueue events but the
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in
'do_mark_muted_user_messages_as_read' which can lead to a
situation where we enqueue events but the function fails at a
later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
In 'do_mark_all_as_read', the transactions which mark the messages
as read in batches should be marked as durable to avoid addition
of any outer atomic block as we support marking a few batches
(not all messages) as read in the case of a timeout.
Earlier, we were using 'send_event' in do_mark_stream_messages_as_read
codepath which can lead to a situation where we enqueue events but the
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
'do_update_message' is within a db transaction, this commit
updates the 'do_clear_mobile_push_notifications_for_ids' function
used in 'do_update_message' to queue event on commit.
Events should not be sent until we know we're not rolling back,
otherwise it can lead to a situation where we enqueue events but
the function fails at a later stage.
Earlier, we were using 'send_event' in
'notify_realm_custom_profile_fields' which can lead to a situation,
if any db operation is added after the 'send_event' in the action
functions using it, where we enqueue event but the action function
fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
The database operations in 'access_user_group_for_setting' and
'check_add_user_group' used in 'add_user_group' view should be
collectively atomic.
This commit adds transaction.atomic decorator for that purpose.
Earlier, we were using 'send_event' in 'delete_user_grou' codepath
which can lead to a situation, if any db operation is added after
the 'send_event' in future, where we enqueue events but the
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in 'edit_user_group' codepath
which can lead to a situation where we enqueue events but the
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in 'do_set_zoom_token' which
can lead to a situation, if any db operation is added after the
'send_event' in future, where we enqueue events but the action
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in 'delete_scheduled_messages'
which can lead to a situation, if any db operation is added after
the 'send_event' in future, where we enqueue events but the action
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in 'edit_scheduled_messages'
which can lead to a situation, if any db operation is added after
the 'send_event' in future, where we enqueue events but the action
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in
'try_deliver_one_scheduled_messages' which can lead to a situation
where we enqueue events but the action function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in 'do_mark_onboarding_step_as_read'
which can lead to a situation, if any db operation is added after the
'send_event' in future, where we enqueue events but the action
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Messages are rendered outside of a transaction, for performance
reasons, and then sent inside of one. This opens thumbnailing up to a
race where the thumbnails have not yet been written when the message
is rendered, but the message has not been sent when thumbnailing
completes, causing `rewrite_thumbnailed_images` to be a no-op and the
message being left with a spinner which never resolves.
Explicitly lock and use he ImageAttachment data inside the
message-sending transaction, to rewrite the message content with the
latest information about the existing thumbnails.
Despite the thumbnailing worker taking a lock on Message rows to
update them, this does not lead to deadlocks -- the INSERT of the
Message rows happens in a transaction, ensuring that either the
message rending blocks the thumbnailing until the Message row is
created, or that the `rewrite_thumbnailed_images` and Message INSERT
waits until thumbnailing is complete (and updated no Message rows).
Earlier, we were using 'send_event' in 'do_schedule_messages' which
can lead to a situation, if any db operation is added after the
'send_event' in future, where we enqueue events but the action
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Earlier, we were using 'send_event' in 'do_update_user_status' which
can lead to a situation, if any db operation is added after the
'send_event' in future, where we enqueue events but the action
function fails at a later stage.
Events should not be sent until we know we're not rolling back.
Fixes part of #30489.
Prakhar Pratyush
Alex Vandiver
Lauryn Menard
Anders Kaseorg
Tim Abbott
Kenneth Rodrigues
Ujjawal Modi
Sahil Batra
evykassirer
roanster007
Prakhar Pratyush