-
Notifications
You must be signed in to change notification settings - Fork 6
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
1.1.0 features #38
1.1.0 features #38
Conversation
@staabm as a first step I gathered the PRs as one here. Tests pass and Packagist has a version for testing. |
@staabm My first test on a Drupal site which also does GDPR transforms on certain fields. I use Orbstack on M1 Pro Mac. Version 1.0.2: 40s Wow! |
Sounds perfect. I will also do some more testing this week |
|| !$this->settings->isEnabled('extended-insert')) { | ||
$onlyOnce = true; | ||
$lineSize = $this->write(';' . PHP_EOL); | ||
$this->write($line . ';' . PHP_EOL); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we are buffering rows into batches with this PR, it might make sense to emit a event at this point in time, so a possible progress indicator could also update when a table dump is in-flight.
otherwise dumping huge tables will not see any progress updates for - in our case - 20-30 minutes.
see also #39
I would propose something like
$this->write($line . ';' . PHP_EOL); | |
$this->write($line . ';' . PHP_EOL); | |
($this->infoCallable)('table', ['name' => $tableName, 'rowProgress' => $count]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another call before the foreach loop is also needed, otherwise the dump info will display an incorrect table name until the 1st million characters were processed.
Also maybe adding a state instead of rowProgress, e.g.:
Before the start of the foreach:
['name' => $tableName, 'completed' => false, 'rowCount' => 0]
Within the foreach loop:
['name' => $tableName, 'completed' => false, 'rowCount' => $count]
After the foreach loop:
['name' => $tableName, 'completed' => true, 'rowCount' => $count]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
valid points. I like your idea more than mine
Would be great this could be merged so I could work on other stuff |
Hi, I’m in bed with flu or something so my countribution will go to next week 🤒 |
All the best. Get well soon |
This reverts commit 9795f14.
New release available https://github.com/druidfi/mysqldump-php/releases/tag/1.1.0 |
Thank you 🙏 |
Fyi, the remaining perf bottleneck in |
This branch can be installed via Packagist: https://packagist.org/packages/druidfi/mysqldump-php#dev-1.1.0-features
composer require druidfi/mysqldump-php:dev-1.1.0-features
or
composer require "druidfi/mysqldump-php:dev-1.1.0-features as 1.0.2"