Looks like fix lies around line 8345 in /oamappps/mcsadmin/mcsadmin.cpp in function printModuleDisk(ModuleDisk moduledisk).

However, from looking at the code, it almost seems as if everything is fixed width, so if a "Device name" is longer than 31?, then this will occur.

void printModuleDisk(ModuleDisk moduledisk)

{

	Oam oam;

	cout << "Module '" + moduledisk.ModuleName + "' Disk Usage (in 1K blocks)" << endl << endl;

	cout << "Mount Point                    Total Blocks  Used Blocks   Usage %" << endl;

	cout << "-----------------------------  ------------  ------------  -------" << endl;

	string etcdir = startup::StartUp::installDir() + "/etc";

	for( unsigned int i = 0 ; i < moduledisk.diskusage.size(); i++)

	{

		//skip mounts to other server disk

		if ( moduledisk.diskusage[i].DeviceName.find("/mnt", 0) == string::npos &&

				moduledisk.diskusage[i].DeviceName.find(etcdir, 0) == string::npos ) {

			cout.setf(ios::left);

			cout.width(31);

			cout << moduledisk.diskusage[i].DeviceName;

			cout.width(14);

			cout << moduledisk.diskusage[i].TotalBlocks;

			cout.width(17);

			cout << moduledisk.diskusage[i].UsedBlocks;

			cout.width(2);

			cout << moduledisk.diskusage[i].DiskUsage << endl;

		}

	}

	cout << endl;

}

I'm not sure if you can just add another space in front of TotalBlocks (which is just after DeviceName), if a check on the length of DeviceName should be performed, or if this should be more dynamic in nature.

I was thinking something like "if length > 28 do '...right 28 bytes of string'" to indicate a truncation and the most significant part still showing.

Assigning to Hill as it would be better for him to decide how this should look.

Adding this to line 8345 should work (fixed how Andrew suggested):

if (moduledisk.diskusage[i].DeviceName.length() > 29) { moduledisk.diskusage[i].DeviceName = "..." + moduledisk.diskusage[i].DeviceName.substr(moduledisk.diskusage[i].DeviceName.length()-26); }

Diff:

C:\mysql\mariadb-columnstore\oamapps\mcsadmin>diff mcsadmin.cpp mcsadmin2.cpp

8344a8345

> if (moduledisk.diskusage[i].DeviceName.length() > 29) { moduledisk.diskusage[i].DeviceName = "..." + moduledisk.diskusage[i].DeviceName.substr(moduledisk.diskusage[i].DeviceName.length()-26); }

I'd love to be able to contribute a patch, officially, so please let me know what I can do to make this happen!

And/or if you'd prefer this be fixed in a different manner, please let me know.

This changed the output from:

Mount Point                    Total Blocks  Used Blocks   Usage %

-----------------------------  ------------  ------------  -------

usr/local/mariadb/columnstore/data1412718256     20634768         5

To:

Mount Point                    Total Blocks  Used Blocks   Usage %

-----------------------------  ------------  ------------  -------

.../mariadb/columnstore/data1  412718256     20634768         5

Note that I opted for the 1-line fix over a more readable if/else. I'd be happy to change it to whatever you prefer, of course.

Hi Chris,

Absolutely you can contribute. You need a github account to do this:

1. Go to https://github.com/mariadb-corporation/mariadb-columnstore-engine
2. Click the "Fork" button in the top right to make a fork into your account
3. Use "git clone" to download your clone (there is a green "Clone or download" button to help on GitHub)
4. "git checkout develop" (this is the 1.1 development tree, we can backport to 1.0)
5. "git checkout -b MCOL-699" (this creates a new local branch based off develop)
6. Make your changes
7. "git commit -a" - make sure you put MCOL-699 at the beginning of the commit message
8. "git push --set-upstream origin MCOL-699" - to push to your GitHub tree
9. Go back to https://github.com/mariadb-corporation/mariadb-columnstore-engine
10. You should see your tree near the top with an option to create a Pull Request with it, press the button to do it
11. At the top left change the base branch to "develop"
12. Hit the green "create pull request" and you are done

If you get stuck at any point please let us know.

As for preference. We would prefer an 'if' to be broken up into multi lines for readability if possible.

Andrew - awesome, many thanks for all of the feedback! Most appreciated

Okay, for the if/else, then this should work in place of line 8345:

if (moduledisk.diskusage[i].DeviceName.length() > 29) { 

	cout << "..." + moduledisk.diskusage[i].DeviceName.substr(moduledisk.diskusage[i].DeviceName.length()-26);

} else {

	cout << moduledisk.diskusage[i].DeviceName;

}

I'll have to start working on the github portion now.

Review request for Chris' pull request which has the behaviour indicated in his comments.

pull request reviewed

also back ported to 1.0.10

commit d5078663c8f9c7fb80ff7a4be2bcc4082df74444
Author: David Hill <david.hill@mariadb.com>
Date: Mon May 15 10:15:16 2017 -0500

MCOL-699 - fix getdisks output

oamapps/mcsadmin/mcsadmin.cpp | 6 +++++-
1 file changed, 5 insertions, 1 deletion

testing on amazon with ebs storage...

reproduced the issue in 1.0.9, will install and test in 1.0.10

mcsadmin> getSystemDisk
getsystemdisk Mon May 22 20:45:19 2017

System Disk Usage per Module

Module 'pm1' Disk Usage (in 1K blocks)

Mount Point Total Blocks Used Blocks Usage %
----------------------------- ------------ ------------ -------
/home/mariadb-user/mariadb/columnstore/data10 0 0

fixed

mcsadmin> getSystemDisk
getsystemdisk Mon May 22 21:18:00 2017

System Disk Usage per Module

Module 'pm1' Disk Usage (in 1K blocks)

Mount Point Total Blocks Used Blocks Usage %
----------------------------- ------------ ------------ -------
.../mariadb/columnstore/data1 103212320 122296 1