-
Notifications
You must be signed in to change notification settings - Fork 41
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
Executor and Parser for onboarding MongoDB #213
Conversation
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.
Sorry just saw this is only a draft, posted a few comments
src/VirtualClient/VirtualClient.Actions.UnitTests/MongoDB/MongoDBMetricsParserTests.cs
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,32 @@ | |||
namespace VirtualClient.Actions.MongoDB | |||
{ | |||
using NUnit.Framework; |
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.
Please add unit test for the executor
"Parameters": {}, | ||
"Actions": [ | ||
{ | ||
"Type": "MongoDBExecutor", |
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.
mongo db is an important workload that we would like to run server/client scenario. Please add that
src/VirtualClient/VirtualClient.Main/profiles/PERF-MONGODB.json
Outdated
Show resolved
Hide resolved
"Parameters": { | ||
"Scenario": "InstallYCSBPackage", | ||
"BlobContainer": "packages", | ||
"BlobName": "ycsb-0.5.0.zip", |
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.
I saw these three benchmarking tool in MongoDB's own blog. https://www.mongodb.com/blog/post/performance-best-practices-benchmarking
Have you research the other 2?
src/VirtualClient/VirtualClient.Main/profiles/PERF-MONGODB.json
Outdated
Show resolved
Hide resolved
src/VirtualClient/VirtualClient.Actions/MongoDB/MongoDBExecutor.cs
Outdated
Show resolved
Hide resolved
…t variable, unable to convert ycsb as an executable file Parameterized the executor commands
[SetUp] | ||
public void Setup() | ||
{ | ||
|
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.
nit: remove extra whitespace
} | ||
|
||
public string GetMongoProcessPath => base.MongoDBPackagePath; | ||
|
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.
same here, remove extra white space
[Test] | ||
public void MongoDBMetricsParserParsesAsExpected() | ||
{ | ||
this.testParser.Parse(); |
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.
Here please check each metrics individually
protected string MongoDBPackagePath { get; set; } | ||
|
||
/// <summary> | ||
/// The file path for YCSB workloads. |
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.
Please update the summary appropriately.
|
||
this.YcsbPackagePath = ycsbPackage.Path; | ||
|
||
// string pathtobin = this.PlatformSpecifics.Combine(this.PackageDirectory, "bin"); |
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.
NIT: Remove extra lines and unnecessary comments.
|
||
this.SetEnvironmentVariable(EnvironmentVariable.JAVA_HOME, javaExecutablePath, EnvironmentVariableTarget.Process); | ||
|
||
await this.ExecuteCommandAsync("chmod", convertToExeArgument, ycsbExecutablePath, telemetryContext, cancellationToken).ConfigureAwait(false); |
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.
We have a MakeFileExecutable function which does the same as part of systemManager. Reuse the same to avoid redundancy.
systemManager.MakeFileExecutableAsync()
/*./ycsb run mongodb -s -P /home/azureuser/vc/content/linux-x64/packages/ycsb/ycsb-0.5.0/workloads/workloada -p operationcount=1000000 -p recordcount=1000000 -threads 16 | ||
*/ | ||
DateTime runStartTime = DateTime.UtcNow; | ||
var runOutput = await this.ExecuteCommandAsync($"{ycsbPath}", updateDataArgument, this.YcsbPackagePath, telemetryContext, cancellationToken) |
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.
load resembles creation of database and run resembles actual execution of transactions between client and server. In benchmarking MongoDB, do we collect information also from load function?
} | ||
|
||
/// <summary> | ||
/// Logic to parse and read metrics8 |
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.
NIT: spell
RegexOptions options = RegexOptions.None; | ||
var regex = new Regex("[ ]{2,}", options); | ||
this.PreprocessedText = regex.Replace(this.RawText, " "); | ||
string pattern = @"(?=\[OVERALL\])"; // Pattern to find the position before the word "[OVERALL]" |
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.
Rather than having comment update the string name so that it's self readable and also update the rgx name below created from the pattern string
|
||
private static Dictionary<string, Tuple<string, double>> GetMetricsMap(string metrics) | ||
{ | ||
char seperator = '\n'; |
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.
Use Environment.NewLine to avoid issues on different platforms.
@@ -133,6 +133,7 @@ | |||
<LinuxProfiles Include="profiles\PERF-SQL-POSTGRESQL.json" /> | |||
<LinuxProfiles Include="profiles\PERF-MYSQL-SYSBENCH-OLTP.json" /> | |||
<LinuxProfiles Include="profiles\PERF-REDIS.json" /> |
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.
NIT: indentation
Hey could you help update this? Do we still need this? |
@yangpanMS - We need this. I am working on the client/server scenario. Will update once ready, |
@revathygs hey any updates on this? Could we first onboard this to internal repo? |
@yangpanMS I think the task to include client server scenarios for mongo db are picked up by someone else in VC team |
Closing. Please reopen as needed. |
No description provided.