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

Round 2 for Drupal transaction isolation, fixes #4281 #4351

Merged
merged 3 commits into from Oct 31, 2022

Conversation

rfay
Copy link
Member

@rfay rfay commented Oct 30, 2022

The Problem/Issue/Bug:

How this PR Solves The Problem:

This is an alternate technique for setting Drupal transaction isolation level. I apologize for re-doing it @tyler36.

Manual Testing Instructions:

Install and/or Bring up a variety of Drupal 9.4, 9.5, 10 sites and view admin/reports/status looking for the warning.

Automated Testing Overview:

Related Issue Link(s):

Release/Deployment notes:

@rfay rfay requested review from tyler36 and rpkoller October 30, 2022 17:02
@github-actions
Copy link

Copy link
Collaborator

@rpkoller rpkoller left a comment

Choose a reason for hiding this comment

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

i've did the following steps:

  1. ddev poweroff and replaced the ddev binary
  2. ddev config with an existing drupal 9 project
  3. removed the mysql directory with the .cnf from the projects .ddev directory
  4. made sure that none of the settings.php files contained the isolation level setting neither
  5. ddev start
  6. ddev launch -p and ran the following query SHOW GLOBAL VARIABLES;
  7. searched in the browser window for READ-
  8. found tx_isolation READ-COMMITTED
  9. went also to the status report page there also no warnings or errors and the isolation level also showed READ-COMMITTED there.

so the mysql part works.

then i also wanted to test if postgres still works alongside. did a ddev config for a drupal 9 site. changed the config.yml to postgres 14 and then did

$> ddev start && ddev composer create "drupal/recommended-project:^9.4"

that lead to

Starting postgres... 
Error response from daemon: Head "https://registry-1.docker.io/v2/library/postgres/manifests/14": Get "https://auth.docker.io/token?scope=repository%3Alibrary%2Fpostgres%3Apull&service=registry.docker.io": dial tcp: lookup auth.docker.io on 1.1.1.1:53: read udp 192.168.5.15:54863->1.1.1.1:53: i/o timeout
Unable to pull docker images: exit status 1
Network ddev-postgres_default  Created 
Container ddev-postgres-web  Started 
Container ddev-postgres-db  Started 
Container ddev-postgres-dba  Started 
Starting mutagen sync process... This can take some time. 
Mutagen sync flush completed in 1s.
For details on sync status 'ddev mutagen st postgres -l' 
Container ddev-router  Running 
Successfully started postgres 
Project can be reached at https://postgres.ddev.site https://127.0.0.1:49173 

not sure if that is due to the docker server is down or it is something with the ddev code? will retry in a few minutes

@rpkoller
Copy link
Collaborator

strange. i got the aforementioned error. but i'Ve deleted all postgres images yesterday. so there werent any. but a docker images -a returned:

REPOSITORY                        TAG                       IMAGE ID       CREATED        SIZE
postgres                          14-postgres-built         6c2c7b05ebc1   40 hours ago   414MB
postgres                          14                        3c326264b599   4 days ago     357MB

real strange. but then i did a ddev launchand went through the installer. no complaints. will quickly try postgres 15 which i definitely have not installed yet with a drupal 10.1 install. and see if that error comes again

@rpkoller
Copy link
Collaborator

strange. the only postgres version i had installed was 14. now i started a ddev project with 15.

$> ddev start
Starting postgres15... 
15: Pulling from library/postgres
dd6189d6fc13: Already exists 
e83a243abe4a: Already exists 
e1e5d0e9701b: Already exists 
b8172b349685: Already exists 
83dad09f3014: Already exists 
c4849d0ca437: Already exists 
7642d443be37: Already exists 
a40f15729374: Already exists 
d965a7635506: Already exists 
aa94e54c6bb6: Already exists 
6d955f7fa1ab: Already exists 
3cd6711cf4eb: Already exists 
2899081d3dc1: Already exists 
Digest: sha256:bab8d7be6466e029f7fa1e69ff6aa0082704db330572638fd01f2791824774d8
Status: Downloaded newer image for postgres:15
docker.io/library/postgres:15

now all parts already existed? yesterday when i first tried one part already existed the other had to be downloaded. i wonder why all already exist? is aside the images also some sort of docker cache on my computer??? but everything went fine afterwards. drupal 10.1 with postgres 15 also no problems still.

ok i went ahead and manually checked all docker images available. docker rmi'ed the postgres 14 and 15 and then created another new project with drupal 9 and postgres14 that had the strange error before. now it worked but same like postgres15.

$> ddev start
Starting post... 
14: Pulling from library/postgres
dd6189d6fc13: Already exists 
e83a243abe4a: Already exists 
e1e5d0e9701b: Already exists 
b8172b349685: Already exists 
83dad09f3014: Already exists 
c4849d0ca437: Already exists 
7642d443be37: Already exists 
a40f15729374: Already exists 
0d91bb9f75c8: Already exists 
11d50fc375b9: Already exists 
58770f84c28e: Already exists 
c66c6cdaad21: Already exists 
466f8597e006: Already exists 
Digest: sha256:135c62a8134dcef829a1e4f5568bfae44bcfa2c75659ff948f43c71964366aa4
Status: Downloaded newer image for postgres:14
docker.io/library/postgres:14

odd that nothing has to be downloaded even though images were deleted. but aside that everything working

@rpkoller
Copy link
Collaborator

is it possible that the already exists statements are due to dockers build cache? did a docker system df -v and foudn several entries in that build cache section i completely forgot.

@tyler36
Copy link
Collaborator

tyler36 commented Oct 31, 2022

I like this approach. Although I think it hides the settings deep within DDEV code and not obvious to new Drupal if/where it is set.

I would like to suggest we display a message when we set it. Something like the following:

Starting d9-demo... 
....
'TRANSACTION ISOLATION LEVEL' set.
Network ddev-d9-demo_default  Created 
Container ddev-d9-demo-web  Started 
Starting d9-demo... 
....
'pg_trgm' extension enabled.
Network ddev-d9-demo_default  Created 
Container ddev-d9-demo-web  Started 

Not sure if this makes it "too noisy" though.

@rfay
Copy link
Member Author

rfay commented Oct 31, 2022

@rpkoller Error response from daemon: Head "https://registry-1.docker.io/v2/library/postgres/manifests/14": Get "https://auth.docker.io/token?scope=repository%3Alibrary%2Fpostgres%3Apull&service=registry.docker.io": dial tcp: lookup auth.docker.io on 1.1.1.1:53: read udp 192.168.5.15:54863->1.1.1.1:53: i/o timeout is purely network trouble, possibly on your end or docker's end, but can also be Colima. When I see too many of these I restart Colima. Colima has a lot of DNS trouble; you're using 1.1.1.1 but still....

As far as docker layer caching... when you delete an image it doesn't mean the cached layers are deleted. But nothing to worry about.

@rfay
Copy link
Member Author

rfay commented Oct 31, 2022

@tyler36 I agree that a message would makes sense... but it would be every single ddev start in that case. I think too noisy. The fact that we're making it easy for people is normal, I think they'll accept that DDEV does lots of work for them. (I even turned off where used to explain about creating settings files and settings permissions over in #4334 - that had been there forever... but somebody thought it was an error message. And everybody else for years was ignoring it.

@rpkoller
Copy link
Collaborator

@rpkoller Error response from daemon: Head "https://registry-1.docker.io/v2/library/postgres/manifests/14": Get "https://auth.docker.io/token?scope=repository%3Alibrary%2Fpostgres%3Apull&service=registry.docker.io": dial tcp: lookup auth.docker.io on 1.1.1.1:53: read udp 192.168.5.15:54863->1.1.1.1:53: i/o timeout is purely network trouble, possibly on your end or docker's end, but can also be Colima. When I see too many of these I restart Colima. Colima has a lot of DNS trouble; you're using 1.1.1.1 but still....

ahhh ok good. and haven't noticed any DNS trouble yet with colima fortunately. but will keep my eyes open in the future. just happened the first time to me.

As far as docker layer caching... when you delete an image it doesn't mean the cached layers are deleted. But nothing to worry about.

yep i've also just noticed that behavior the first time consciously.

and in regards of @tyler36's point. i agree it would be a good thing to be transparent what is done under the hood but i also agree with @rfay that startups might become more of an information overload. but how about adding those details in the docs? add another paragraph to the drupal section in here perhaps? https://ddev.readthedocs.io/en/stable/users/quickstart/#system-requirements or is there a more specific place where all the measures a configuration entails for a cms?

@rfay
Copy link
Member Author

rfay commented Oct 31, 2022

I agree about another paragraph in the Drupal section of the docs. Great idea. Then we don't have to feel guilty. Drupal folk get away with not knowing much about sysadmin when using DDEV.

@tyler36
Copy link
Collaborator

tyler36 commented Oct 31, 2022

Drupal folk get away with not knowing much about sysadmin when using DDEV.

DDEV, it just works tm

@rfay rfay merged commit c6a2fc4 into ddev:master Oct 31, 2022
@rfay rfay deleted the 20221030_drupal_mysql_isolation branch October 31, 2022 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants