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

feat/condo/infra-200/postgres-ha #4824

Closed
wants to merge 68 commits into from

Conversation

Gladioluss
Copy link
Contributor

No description provided.

…ections handling option [SQUASH THIS COMMIT DUE TO TEST CHANGES]
@toplenboren
Copy link
Member

Your branch looks a bit strange, I see some commits from @sitozzz , is this intentional?

@toplenboren
Copy link
Member

Also, if you want review, the best practice is to leave a comment with description: what was done, why, and so on :-)

@sitozzz sitozzz marked this pull request as draft June 24, 2024 06:16
@sitozzz sitozzz self-assigned this Jun 24, 2024
run: |
docker run -p="127.0.0.1:6379:6379" -d ${{ secrets.DOCKER_REGISTRY }}/doma/utils/redis:6.2

- name: run postgresql replica
run: |
docker run --name postgre-replica --user postgres --network=db_network -e PGUSER=$PGUSER -e PGPASSWORD=$PGPASSWORD -e POSTGRES_DB=$POSTGRES_DB -p="127.0.0.1:5433:5432" -d ${{ secrets.DOCKER_REGISTRY }}/doma/utils/postgres:13.2 bash -c "if [ ! -f /var/lib/postgresql/initialized ]; then pg_basebackup --pgdata=/var/lib/postgresql/data -R --slot=replication_slot --host=postgre-master --port=5432; chmod -R 0700 /var/lib/postgresql/data; touch /var/lib/postgresql/initialized; fi; postgres -c max_connections=2000"
Copy link
Member

Choose a reason for hiding this comment

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

Why not just docket compose up -d ?

@@ -51,7 +52,7 @@ node -e 'console.log(v8.getHeapStatistics().heap_size_limit/(1024*1024))'
# NOTE(pahaz): Keystone not in dev mode trying to check dist/admin folder
mkdir -p ./apps/condo/dist/admin

[[ $DATABASE_URL == postgresql* ]] && yarn workspace @app/condo migrate
[[ $DATABASE_URL == postgresql* || $DATABASE_URL == custom* ]] && yarn workspace @app/condo migrate
Copy link
Member

Choose a reason for hiding this comment

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

Why not to run it always?

: checkUseMasterSingle(sql)
} catch (error) {
logger.error({
err: error,
Copy link
Member

Choose a reason for hiding this comment

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

May be we need to add sql to logging

run: |
docker run -e POSTGRES_USER=$POSTGRES_USER -e POSTGRES_PASSWORD=$POSTGRES_PASSWORD -e POSTGRES_DB=$POSTGRES_DB -p="127.0.0.1:5432:5432" -d ${{ secrets.DOCKER_REGISTRY }}/doma/utils/postgres:13.2
touch ./pg_hba.conf
Copy link
Member

Choose a reason for hiding this comment

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

Just docker compose up

Copy link
Member

Choose a reason for hiding this comment

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

That's not a main branch. Please add commits directly to the main task #4618

Copy link
Member

Choose a reason for hiding this comment

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

For now self-hosted runner is unable to run compose services. In master branch we already using same service deployment strategy, so your comment is unrelated to this PR. Runner will be updated and after that I'll fix that

@sitozzz
Copy link
Member

sitozzz commented Jun 24, 2024

Close this PR due to duplication of the original one #4618

@sitozzz sitozzz closed this Jun 24, 2024
Copy link

sonarcloud bot commented Jun 24, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants