Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Move Complement test matrix jobs definition to match Sytest and Trial. #14153

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
d7192cd
Duplicate worker types defined in start_for_complement.sh into calcul…
realtyem Oct 12, 2022
5847f41
Modify tests.yml to use the new job matrix.
realtyem Oct 12, 2022
db446cc
Pass the SYNAPSE_WORKER_TYPES through to the Complement tests.
realtyem Oct 12, 2022
560e28c
Strip out the now unneeded part of start_for_complement.sh.
realtyem Oct 12, 2022
84399a3
Changelog.
realtyem Oct 12, 2022
2f9f79b
Merge branch 'develop' into realtyem/complement-move-workers-def-to-o…
realtyem Oct 12, 2022
6be11b6
Adjust for PR #14028 being merged.
realtyem Oct 12, 2022
a246dda
Fix executable permission on start_for_complement.sh.
realtyem Oct 12, 2022
01992aa
Don't actually need to put the SYNAPSE_ prefix on env variables.
realtyem Oct 12, 2022
785cf2e
Update docs and include example.
realtyem Oct 12, 2022
a4306d1
Merge branch 'develop' into realtyem/complement-move-workers-def-to-o…
realtyem Oct 12, 2022
a5de665
Merge branch 'develop' into realtyem/complement-move-workers-def-to-o…
realtyem Oct 16, 2022
079c5d9
Create backwards compatibility use for WORKERS=1 in start_for_complem…
realtyem Oct 16, 2022
7528511
[REVERT THIS] Create quick test to make sure it all works as intended.
realtyem Oct 16, 2022
6846a1c
How did those get in there?
realtyem Oct 17, 2022
80afb8a
Update to docs.
realtyem Oct 17, 2022
eca4595
Merge branch 'develop' into realtyem/complement-move-workers-def-to-o…
realtyem Oct 21, 2022
b16d4b3
It's an underscore, not a hyphen
realtyem Oct 21, 2022
6c8675c
Seriously? Editing on Github directly changes the executable permissi…
realtyem Oct 21, 2022
720f55e
Merge branch 'develop' into realtyem/complement-move-workers-def-to-o…
realtyem Oct 25, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions .ci/scripts/calculate_jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,63 @@ def set_output(key: str, value: str):

test_matrix = json.dumps(sytest_tests)
set_output("sytest_test_matrix", test_matrix)


# First calculate the workers used during complement jobs
#
# Will only be used on tests with SYNAPSE_COMPLEMENT_USE_WORKERS enabled
# Github sees a none existent key and evaluates it to an empty string, according to
# https://docs.github.com/en/actions/learn-github-actions/contexts
# See the last sentence before "Determining when to use contexts"

complement_test_jobs = [
{"arrangement": "monolith", "database": "SQLite"},
{"arrangement": "monolith", "database": "Postgres"},
{
"arrangement": "workers",
"database": "Postgres",
"worker_types": ", ".join(
worker
for worker in [
"event_persister",
"event_persister",
"background_worker",
"frontend_proxy",
"event_creator",
"user_dir",
"media_repository",
"federation_inbound",
"federation_reader",
"federation_sender",
"synchrotron",
"client_reader",
"appservice",
"pusher",
]
),
},
]

# Testing for backwards compatibility mode. It's a simple matter of just NOT putting in
# a worker_types string. It will resolve to an empty string on the other side.

complement_backwards_compatibility_tests = [
{
"database": "Postgres",
"arrangement": "workers",
}
]

print("::group::Calculated complement job with workers")
print(json.dumps(complement_test_jobs, indent=4))
print("::endgroup::")

print("::group::Calculated backwards compatibility complement job with workers")
print(json.dumps(complement_backwards_compatibility_tests, indent=4))
print("::endgroup::")

test_matrix = json.dumps(complement_test_jobs)
set_output("complement_test_matrix", test_matrix)

test_matrix = json.dumps(complement_backwards_compatibility_tests)
set_output("complement_backwards_compatibility_test_matrix", test_matrix)
59 changes: 47 additions & 12 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ jobs:
outputs:
trial_test_matrix: ${{ steps.get-matrix.outputs.trial_test_matrix }}
sytest_test_matrix: ${{ steps.get-matrix.outputs.sytest_test_matrix }}
complement_test_matrix: ${{ steps.get-matrix.outputs.complement_test_matrix }}
complement_backwards_compatibility_test_matrix: ${{ steps.get-matrix.outputs.complement_backwards_compatibility_test_matrix }}

trial:
if: ${{ !cancelled() && !failure() }} # Allow previous steps to be skipped, but not fail
Expand Down Expand Up @@ -436,21 +438,50 @@ jobs:

complement:
if: "${{ !failure() && !cancelled() }}"
needs: linting-done
needs: calculate-test-jobs
runs-on: ubuntu-latest

strategy:
fail-fast: false
matrix:
include:
- arrangement: monolith
database: SQLite
job: ${{ fromJson(needs.calculate-test-jobs.outputs.complement_test_matrix) }}

steps:
- name: Run actions/checkout@v3 for synapse
uses: actions/checkout@v3
with:
path: synapse

- arrangement: monolith
database: Postgres
- name: Install Rust
uses: actions-rs/toolchain@v1
with:
toolchain: 1.58.1
override: true
- uses: Swatinem/rust-cache@v2

- arrangement: workers
database: Postgres
- name: Prepare Complement's Prerequisites
run: synapse/.ci/scripts/setup_complement_prerequisites.sh

- name: Run Complement Tests
shell: bash
env:
POSTGRES: ${{ (matrix.job.database == 'Postgres') && 1 || '' }}
WORKERS: ${{ (matrix.job.arrangement == 'workers') && 1 || '' }}
WORKER_TYPES: ${{ matrix.job.worker_types }}
run: |
set -o pipefail
COMPLEMENT_DIR=`pwd`/complement synapse/scripts-dev/complement.sh -json 2>&1 | synapse/.ci/scripts/gotestfmt

back-compat:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wouldn't this be included under the complement job above as part of the matrix?

It's also not clear what this is testing backwards compatibility with?

name: "Backwards Compatibility Test"
if: "${{ !failure() && !cancelled() }}"
needs: calculate-test-jobs
runs-on: ubuntu-latest

strategy:
fail-fast: false
matrix:
job: ${{ fromJson(needs.calculate-test-jobs.outputs.complement_backwards_compatibility_test_matrix) }}

steps:
- name: Run actions/checkout@v3 for synapse
Expand All @@ -468,11 +499,15 @@ jobs:
- name: Prepare Complement's Prerequisites
run: synapse/.ci/scripts/setup_complement_prerequisites.sh

- run: |
set -o pipefail
POSTGRES=${{ (matrix.database == 'Postgres') && 1 || '' }} WORKERS=${{ (matrix.arrangement == 'workers') && 1 || '' }} COMPLEMENT_DIR=`pwd`/complement synapse/scripts-dev/complement.sh -json 2>&1 | synapse/.ci/scripts/gotestfmt
- name: Run Complement Tests
shell: bash
name: Run Complement Tests
env:
POSTGRES: ${{ (matrix.job.database == 'Postgres') && 1 || '' }}
WORKERS: ${{ (matrix.job.arrangement == 'workers') && 1 || '' }}
WORKER_TYPES: ${{ matrix.job.worker_types }}
run: |
set -o pipefail
COMPLEMENT_DIR=`pwd`/complement synapse/scripts-dev/complement.sh -json 2>&1 | synapse/.ci/scripts/gotestfmt

cargo-test:
if: ${{ needs.changes.outputs.rust == 'true' }}
Expand Down
1 change: 1 addition & 0 deletions changelog.d/14153.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Move Complement CI job matrix definition to central location to match Sytest and Trial. Contributed by Jason Little.
15 changes: 12 additions & 3 deletions docker/complement/conf/start_for_complement.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ set -e

echo "Complement Synapse launcher"
echo " Args: $@"
echo " Env: SYNAPSE_COMPLEMENT_DATABASE=$SYNAPSE_COMPLEMENT_DATABASE SYNAPSE_COMPLEMENT_USE_WORKERS=$SYNAPSE_COMPLEMENT_USE_WORKERS"
echo " Env: SYNAPSE_COMPLEMENT_DATABASE=$SYNAPSE_COMPLEMENT_DATABASE"
echo " Env: SYNAPSE_COMPLEMENT_USE_WORKERS=$SYNAPSE_COMPLEMENT_USE_WORKERS"
echo " Env: SYNAPSE_WORKER_TYPES=$SYNAPSE_WORKER_TYPES"
echo " NOTE: if using the backward compatibility mode, worker types aren't declared yet."

function log {
d=$(date +"%Y-%m-%d %H:%M:%S,%3N")
Expand Down Expand Up @@ -45,7 +48,12 @@ esac

if [[ -n "$SYNAPSE_COMPLEMENT_USE_WORKERS" ]]; then
# Specify the workers to test with
realtyem marked this conversation as resolved.
Show resolved Hide resolved
export SYNAPSE_WORKER_TYPES="\
# Allow overriding by explicitly setting SYNAPSE_WORKER_TYPES outside, while still
# utilizing WORKERS=1 for backwards compatibility.
# -n True if the length of string is non-zero.
# -z True if the length of string is zero.
if [[ -z "$SYNAPSE_WORKER_TYPES" ]]; then
export SYNAPSE_WORKER_TYPES="\
event_persister, \
event_persister, \
background_worker, \
Expand All @@ -57,10 +65,11 @@ if [[ -n "$SYNAPSE_COMPLEMENT_USE_WORKERS" ]]; then
federation_reader, \
federation_sender, \
synchrotron, \
client_reader, \
appservice, \
pusher"

log "Workers now declared: $SYNAPSE_WORKER_TYPES"
fi
# Improve startup times by using a launcher based on fork()
export SYNAPSE_USE_EXPERIMENTAL_FORKING_LAUNCHER=1
else
Expand Down
3 changes: 2 additions & 1 deletion docs/development/contributing_guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,8 @@ COMPLEMENT_DIR=../complement ./scripts-dev/complement.sh -run TestImportHistoric
The above will run a monolithic (single-process) Synapse with SQLite as the database. For other configurations, try:

- Passing `POSTGRES=1` as an environment variable to use the Postgres database instead.
- Passing `WORKERS=1` as an environment variable to use a workerised setup instead. This option implies the use of Postgres.
- Passing `WORKERS=1` as an environment variable to use a set of workers that mirrors what is used in Sytest. This option implies the use of Postgres.
- If setting `WORKERS=1`, optionally set `WORKER_TYPES=` to declare which worker types you wish to test. A simple comma-delimited string containing the worker types defined from the template in [here](https://github.com/matrix-org/synapse/blob/develop/docker/configure_workers_and_start.py). A safe example would be `WORKER_TYPES="federation_inbound,federation_sender,synchrotron,"`. See the [worker documentation](https://matrix-org.github.io/synapse/latest/workers.html) for additional information on workers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you don't want to have a trailing comma after synchrotron?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, that's right it needs to not be there.


To increase the log level for the tests, set `SYNAPSE_TEST_LOG_LEVEL`, e.g:
```sh
Expand Down
3 changes: 3 additions & 0 deletions scripts-dev/complement.sh
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ if [[ -n "$WORKERS" ]]; then
# Use workers.
export PASS_SYNAPSE_COMPLEMENT_USE_WORKERS=true

# Pass through the workers defined. If none, it will be an empty string
export PASS_SYNAPSE_WORKER_TYPES="$WORKER_TYPES"
Copy link
Contributor Author

@realtyem realtyem Oct 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way it matches with the variables being declared on the commandline and environment at the same time. Standardization is good, redundancy is bad.


# Workers can only use Postgres as a database.
export PASS_SYNAPSE_COMPLEMENT_DATABASE=postgres

Expand Down