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

Enforce event ordering #1103

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Arnei
Copy link
Member

@Arnei Arnei commented Feb 3, 2025

WILL BREAK THE UI WITHOUT THE REQUIRED BACKEND CHANGES: opencast/opencast#6461

The events table is sorted by a sort parameter. By default, this is the start date. If there are multiple events with the same start date, the sorting for those is left to the elasticseach index in the backend. Elasticsearch however does not guarantee any ordering. This may result in the order of events changing unexpectedly like in #1102.

This patch aims to solve this by adding a secondary sort parameter to the events query. The primary parameter still takes priority. For the secondary parameter, uid was chosen as it is the only field guaranteed to be unique.

More sorting parameters can lead to increased query times. If someone with an Opencast with many events could test the impact of these changes on the events.json request times, that would be great.

How to test this

Requires an Opencast with the PR opencast/opencast#6461 installed.

**WILL BREAK THE UI WITHOUT THE REQUIRED BACKEND CHANGES:** opencast/opencast#6461

The events table is sorted by a sort parameter. By default, this is
the start date. If there are multiple events with the same start date,
the sorting for those is left to the elasticseach index in the
backend. Elasticsearch however does not guarantee any ordering.
This may result in the order of events changing unexpectedly like
in opencast#1102.

This patch aims to solve this by adding a secondary
sort parameter to the
events query. The primary parameter still takes priority.
For the secondary parameter, `uid` was chosen as it is the only
field guaranteed to be unique.

More sorting parameters can lead to increased query times.
If someone with *many* events could test
the impact of these changes
that would be great.
@Arnei Arnei added the type:bug Something isn't working label Feb 3, 2025
Copy link
Contributor

github-actions bot commented Feb 3, 2025

Use docker or podman to test this pull request locally.

Run test server using develop.opencast.org as backend:

podman run --rm -it -p 127.0.0.1:3000:3000 ghcr.io/opencast/opencast-admin-interface:pr-1103

Specify a different backend like stable.opencast.org:

podman run --rm -it -p 127.0.0.1:3000:3000 -e PROXY_TARGET=https://stable.opencast.org ghcr.io/opencast/opencast-admin-interface:pr-1103

It may take a few seconds for the interface to spin up.
It will then be available at http://127.0.0.1:3000.
For more options you can pass on to the proxy, take a look at the README.md.

Copy link
Contributor

github-actions bot commented Feb 3, 2025

This pull request is deployed at test.admin-interface.opencast.org/1103/2025-02-03_10-32-21/ .
It might take a few minutes for it to become available.

@owi92
Copy link
Contributor

owi92 commented Feb 7, 2025

I haven't tested the response times, only the ordering. While the specific issue of #1102 seems to be fixed, I can sill observe some jumping when randomly clicking filters (see screencapture). So in regards to

Sort the events table by different parameters and check if the resulting sorting make sense.

I wouldn't say that it does. (I tested with opencast/opencast#6461).

Bildschirmaufnahme.2025-02-07.um.12.22.37.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants