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.

Sign up for GitHub

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

Jump to bottom

Users/nmalkapuram/my sql compete recipe #403

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Users/nmalkapuram/my sql compete recipe #403

wants to merge 10 commits into from

Conversation

nmalkapuram
Copy link
Contributor

@nmalkapuram nmalkapuram commented Nov 15, 2024 •
edited
Loading

Features:
Adding truncate database option to enable compete recipe for MYSQL.
Bug Fixes:
updated config files to not maintain replication logs, updated actual global values used to be consistent with the values passed in the profile.

yangpanMS
yangpanMS requested changes Nov 15, 2024
View reviewed changes
Copy link
Contributor

@yangpanMS yangpanMS left a comment

Choose a reason for hiding this comment

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

When you say recipe is that for ABC recipe? When you make behavior changes could you reflect the changes in the mysql document to explain what we are changing

@nmalkapuram nmalkapuram force-pushed the users/nmalkapuram/MySQLCompeteRecipe branch 2 times, most recently from 38a046b to d101531 Compare November 19, 2024 03:28
@nmalkapuram nmalkapuram marked this pull request as draft November 25, 2024 17:12
@nmalkapuram nmalkapuram force-pushed the users/nmalkapuram/MySQLCompeteRecipe branch from d101531 to b162834 Compare November 29, 2024 07:46
@nmalkapuram nmalkapuram marked this pull request as ready for review November 29, 2024 08:38
@nmalkapuram nmalkapuram force-pushed the users/nmalkapuram/MySQLCompeteRecipe branch from 9e9129b to 03f1b17 Compare December 2, 2024 10:45
brdeyo
brdeyo requested changes Dec 2, 2024
View reviewed changes
src/VirtualClient/VirtualClient.Contracts/EnvironmentLayoutExtensions.cs Outdated Show resolved Hide resolved
src/VirtualClient/VirtualClient.Actions/Sysbench/SysbenchConfiguration.cs
@@ -81,6 +84,13 @@ private async Task PrepareOLTPMySQLDatabase(EventContext telemetryContext, Cance

this.sysbenchPrepareArguments = $"--dbName {this.DatabaseName} --databaseSystem {this.DatabaseSystem} --benchmark {this.Benchmark} --tableCount {tableCount} --recordCount {recordCount} --threadCount {threadCount} --password {this.SuperUserPassword}";

if (this.IsMultiRoleLayout())
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed? The SysbenchConfiguration SHOULD be running on the Server role (database) system to begin with.

src/VirtualClient/VirtualClient.Actions/CtsTraffic/CtsTrafficClientExecutor.cs
@@ -98,7 +99,8 @@ await this.ServerApiClient.PollForExpectedStateAsync<CtsTrafficServerState>(
cancellationToken,
this.PollingInterval);

string targetIPAddress = this.GetServerIPAddress(cancellationToken);
ClientInstance instance = this.Layout.GetClientInstance(this.AgentId);
Copy link
Contributor

Choose a reason for hiding this comment

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

As mentioned in other comments, there is an extension method on the VirtualClientComponent class that can get instances by role. We don't need the GetServerIpAddress method.

src/VirtualClient/VirtualClient.Actions/Sysbench/SysbenchClientExecutor.cs
DependencyPath package = await this.GetPlatformSpecificPackageAsync(this.PackageName, cancellationToken);
this.packageDirectory = package.Path;

await this.TruncateMySQLDatabaseAsync(telemetryContext, cancellationToken).ConfigureAwait(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we doing this? The logic to create the database is already a part of the MySqlServerConfiguration and SysbenchConfiguration classes. We ARE NOT trying to support rebuilding the database on every round of processing. This is extremely inefficient.

It may not be clear how we intend to support this database workload, so I will share the guidance given to Erica initially. The related profile should build the database out on a first run. It does this making no assumptions and will/should drop the database if it exists. The database should then be reused on each subsequent run. This means that the database must be a set size (e.g. #tables, records) for a given profile. If the user decides they want to operate against a database that is sized differently, they are expected to perform a "clean". This removes the state tracking and causes VC to behave like it is a first time run again thus dropping any existing DB and recreating it. I don't want to attempt to handle deep eccentricities with database workloads such as resizing an existing database in any other way other than dropping and recreating.

@nmalkapuram nmalkapuram force-pushed the users/nmalkapuram/MySQLCompeteRecipe branch from 03f1b17 to c15c2e6 Compare January 6, 2025 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yangpanMS yangpanMS yangpanMS requested changes

@brdeyo brdeyo brdeyo requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nmalkapuram @yangpanMS @brdeyo