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

Support parallel migration #276

Closed
wants to merge 1 commit into from
Closed

Conversation

sanak
Copy link
Member

@sanak sanak commented Apr 15, 2024

Currently, parallel environment migration causes deadlock issue.

== 20171004054800 EnsureProperSrid: migrating =================================
-- execute("SELECT UpdateGeometrySRID('issues','geom',4326)")
   -> 0.0488s
-- execute("SELECT UpdateGeometrySRID('projects','geom',4326)")
   -> 0.0091s
-- execute("SELECT UpdateGeometrySRID('users','geom',4326)")
rake aborted!
StandardError: An error has occurred, this and all later migrations canceled:

PG::TRDeadlockDetected: ERROR:  deadlock detected
DETAIL:  Process 128 waits for AccessExclusiveLock on relation 19995 of database 16384; blocked by process 127.
Process 127 waits for AccessShareLock on relation 19949 of database 16384; blocked by process 128.
HINT:  See server log for query details.
CONTEXT:  SQL statement "ALTER TABLE public.users ALTER COLUMN geom TYPE  geometry(Geometry, 4326) USING public.ST_SetSRID(geom,4326);"
PL/pgSQL function updategeometrysrid(character varying,character varying,character varying,character varying,integer) line 80 at EXECUTE
SQL statement "SELECT public.UpdateGeometrySRID('','',$1,$2,$3)"
PL/pgSQL function updategeometrysrid(character varying,character varying,integer) line 5 at SQL statement

Changes proposed in this pull request:

  • Support parallel migration by transaction guard
  • CI support?
    => I thought to append bundle exec rake redmine:plugins:migration & bundle exec rake redmine:plugins:migration & wait with 2 processes at the bottom of CI, but parallel execution is a rare case, so I don't support this.

@gtt-project/maintainer

@sanak sanak added the bug Something isn't working label Apr 15, 2024
@sanak sanak self-assigned this Apr 15, 2024
@sanak sanak marked this pull request as ready for review April 17, 2024 07:26
@sanak sanak requested review from smellman and dkastl April 17, 2024 07:26
@smellman
Copy link
Contributor

I think transaction will run in current code(because rollback some migrations), so I need to understand the reason why this patch work well.

Copy link
Member

@dkastl dkastl left a comment

Choose a reason for hiding this comment

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

To me this looks fine. Maybe wrapping each into a transaction will slow down SQL commands a little bit, but for migrations this should not matter really.

So I think let's do it this way.

@sanak
Copy link
Member Author

sanak commented Apr 17, 2024

@smellman (CC: @dkastl)

I think transaction will run in current code(because rollback some migrations), so I need to understand the reason why this patch work well.

Well, the transaction purpose is not for several migrations, but for avoid simultaneousALTER COLUMN ... SQL statement execution in 1 migration. But in this case, there may be another solution, so I will take a look at it.
(I just found similar old issue in rails.)
rails/rails#22092

Also, I will share deadlock reproducible steps in our GitLab.

@dkastl
Copy link
Member

dkastl commented May 24, 2024

Shall we merge this?

@sanak
Copy link
Member Author

sanak commented May 24, 2024

@dkastl Sorry, I forgot to check more things.
I will try to check those by the middle of next week, so please wait for a while. 🙇

@sanak
Copy link
Member Author

sanak commented Jun 2, 2024

Sorry, I am not sure whether this is appropriate solution, so I will close this PR. 🙇
(Note that I added a comment in our GitLab side, so just for your information.)

@sanak sanak closed this Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants