Skip to content

Conversation

@harlor
Copy link

@harlor harlor commented Jul 25, 2025

We've noticed that importing a dump using drush sql:cli is significantly slower than using mysql directly. After some investigation we found out that this seems to be caused by the buffering in PHP.

In a specific use case we measured the following times:

drush sqlc: 3min 39s
mysql: 19s

In this PR we improve the performance by using the passthru() function in case of the non-interactive mode (e.g. piped SQL dump).

@weitzman
Copy link
Member

weitzman commented Jul 25, 2025

Nice find. I see that one of the sites I inherited actually suffers from this exact problem. Our speed difference is more like 50% - not quite as dramatic as yours. I see two possible approaches:

  1. We merge this PR (or something like it) and keep on supporting sql:cli for importing DB dumps
  2. We keep on recommending $(drush sql:connect) < example.sql. Maybe we should forcibly break in the next major version when anything redirects to sql:cli. We almost did that in New TTY detection in 9.7.0 breaks sql-cli input via STDIN (fixed, needs test) #4092. Please read that for backstory.

@weitzman
Copy link
Member

Based on #4092, I summon @danepowell and @greg-1-anderson for an opinion on how to proceed.

@danepowell
Copy link
Contributor

danepowell commented Jul 26, 2025

sql:cli is so problematic, I wouldn't mind seeing it deprecated. If this PR makes life a little easier for folks in the meantime, I don't see a problem with it (I haven't reviewed it technically).

@harlor just curious, long-term, is Moshe's suggestion of using sql:connect in a subshell viable for your use case?

@harlor
Copy link
Author

harlor commented Aug 4, 2025

Oh TBH, I wasn't aware that sql:cli is so problematic. I think sql:connect woud work for for us.

What would be the recommended way to handle (hide) the password? Depending on the version, mysql complains about the command with:

mysql: [Warning] Using a password on the command line interface can be insecure.

@weitzman
Copy link
Member

weitzman commented Aug 4, 2025

That tip has been shown for a decade without any significant concern. My understanding is that MySQL is warning you in case you have a shared server - others can see the DB password briefly on the process list. Shared servers are much less common now with Docker providing isolation for process list and much else.

weitzman added a commit to weitzman/drush that referenced this pull request Aug 4, 2025
@weitzman
Copy link
Member

weitzman commented Aug 4, 2025

I'm inclined to change this just with documentation as per #6333. The fix here is a bit invasive IMO. I added a warning log message when you use the slow approach.

@weitzman weitzman closed this in 635fce5 Aug 4, 2025
@weitzman
Copy link
Member

I am working another large site and it too suffers from using sql:cli. Reopening this to block the slow approahc in Drush 14.

@weitzman weitzman reopened this Sep 24, 2025
@chx
Copy link
Contributor

chx commented Sep 29, 2025

Very interesting indeed! but why is this being fixed in drush instead of symfony/process?

weitzman added a commit that referenced this pull request Nov 12, 2025
* Block piping to sql:cli command

* PHPCS
@weitzman
Copy link
Member

I blocked pipeing to sql:cli in 14.x. See #6436

@weitzman weitzman closed this Nov 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants