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

Show usage per database. #816

Open
wants to merge 11 commits into
base: development
Choose a base branch
from
Open

Show usage per database. #816

wants to merge 11 commits into from

Conversation

matthiaz
Copy link
Contributor

@matthiaz matthiaz commented Jun 27, 2019

Customer reported a bug in the new version. That caused him to see the same numbers for every database/relation. I had to add a new table to show the database sizes because we can't directly compare that against the allocated size.
On the bright side: this does make it very clear where the storage usage is coming from.

image

matthiaz added 3 commits June 27, 2019 14:55
…reported by customer: https://platformsh.zendesk.com/agent/tickets/60817

Fix is a little bigger than I'd like, but I needed to differentiate between the usage vs allocated. And the usage per database. Because else we're comparing apples and oranges.
@pjcdawkins
Copy link
Collaborator

pjcdawkins commented Jun 27, 2019

Customer reported a bug in the new version. That caused him to see the same numbers for every database/relation.

Sorry.. what's the bug?

@matthiaz
Copy link
Contributor Author

I removed the colouring on the database table, because we don't want to alarm the customer for no reason. There is nothing wrong with a customer having a single database ;-)

@matthiaz
Copy link
Contributor Author

Do note that this also fixes a UI bug that was introduced in the previous merge to development.

@pjcdawkins
Copy link
Collaborator

Still scratching my head over what the UI bug is. Do you mean the per-relationship usage? That is/was misleading, as the disk exists independently of the relationships, so you'd need to see the full usage on the disk regardless of which relationship is used. Breaking down the usage per schema/table as well as showing the full usage is a useful feature.

@matthiaz
Copy link
Contributor Author

matthiaz commented Jul 12, 2019 via email

@matthiaz
Copy link
Contributor Author

@pjcdawkins Do you want me to reclassify this as a feature request to merge this?


$this->showInaccessibleSchemas($service, $database);
$db_table = $this->getService('table');
$db_columns = ['db' => 'Database', 'used' => 'Estimated usage', 'percent' => 'Percentage'];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Having two tables and naming the columns in both is going to be awkward, I believe it will break the --columns option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. It does produce weird results. I moved that logic under a flag to make it optional.

@@ -377,12 +418,13 @@ private function getMySqlUsage(HostInterface $host, array $database) {
*
* @return string
*/
private function formatPercentage($percentage, $machineReadable) {
private function formatPercentage($percentage, $machineReadable, $blnUseColour=true) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Irrelevant / unnecessary change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

$blnUseColour : I use it, because we don't want to show colours when were simply listing the distribution of database usage size.
See: platform db:size -p sudo27zgjpwqa -vvv -e WEBSUPPORT-170-office-renewal --relationship=management -D

matthiaz and others added 2 commits September 18, 2019 10:05
…ibution

Otherwise it will produce weird results when using --format and --csv
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.

2 participants