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

composer: add initial version #75

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,5 @@ __pycache__/
# Ignore all local history of files
.history

node_modules/
# End of https://www.gitignore.io/api/go,visualstudiocode
182 changes: 182 additions & 0 deletions composer/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
import sqlite3 from "sqlite3";
import { open } from "sqlite";
Comment on lines +1 to +2
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the difference between these two packages?

Copy link
Member Author

Choose a reason for hiding this comment

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

sqlite is a wrapper library that uses sqlite3 as the driver.

import { promisify } from "util";
import toml from "@iarna/toml";
import yargs from "yargs";

const execFile = promisify(require("node:child_process").execFile);
const writeFile = promisify(require("node:fs").writeFile);
const unlink = promisify(require("node:fs").unlink);

const DB = "combinations.db";
// Set the builder as docker generic for every group.
const BUILDER = "docker:generic";

// Command line arguments.
const argv = yargs(process.argv.slice(2))
.options({
"rust-git-ref": { type: "string", demandOption: false },
"rust-git-target": { type: "string", demandOption: false },
"go-git-ref": { type: "string", demandOption: false },
"go-git-target": { type: "string", demandOption: false },
total_instances: { type: "number", demandOption: true },
})
.parseSync();

// TOML schema to generate.
class Instance {
constructor(public count: number) {}
}

class BuildArgs {
VERSION: string;
GIT_REF?: string;
GIT_TARGET?: string;

constructor(
version: string,
rustGitRef?: string,
rustGitTarget?: string,
goGitRef?: string,
goGitTarget?: string
) {
this.VERSION = version;

if (version.includes("master")) {
if (version.includes("rust")) {
this.GIT_REF = rustGitRef;
this.GIT_TARGET = rustGitTarget;
}
if (version.includes("go")) {
this.GIT_REF = goGitRef;
this.GIT_TARGET = goGitTarget;
}
}
}
}

class BuildConfig {
constructor(public build_args: BuildArgs) {}
}

class TestParams {
constructor(
public transport: string,
public muxer: string,
){}
}

class Group {
constructor(
public id: string,
public builder: string,
public instances: Instance,
public build_config: BuildConfig,
public test_params: TestParams
) {}
}

class Run {
constructor(public id: string, public groups: Group[]) {}
}

class Global {
constructor(
public plan: string,
public plan_case: string,
public total_instances: number
) {}
}

class Composition {
constructor(public global: Global, public runs: Run[]) {}
}

async function main() {
sqlite3.verbose();

// Call sqlite to process the csv resource files and generate a database.
// We call the sqlite process instead of doing it here cause
// the dot commands are interpreted by the sqlite cli tool not sqlite itself,
// and it is a lot faster parsing the csv's.
Copy link
Contributor

Choose a reason for hiding this comment

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

Our CSVs are <10 lines long. What am I meant to take away from this comment?

Copy link
Member Author

@jxs jxs Nov 21, 2022

Choose a reason for hiding this comment

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

that we can't call sqlite dot commands via the database driver, therefore we do it via execFile therefore instead of parsing the csv file with js, we do it with sqlite instead cause it's faster.

Our CSVs are <10 lines long.

but they are expected to grow I assume, do you suggest importing, parsing and inserting in typescript land instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Our CSVs are <10 lines long.

but they are expected to grow I assume

I am guessing they would have to grow into the thousands for performance to become a problem and thus a driving factor for the design. I hope that we won't have CSV files that are 1000s of lines long!

do you suggest importing, parsing and inserting in typescript land instead?

I think we should do whatever results in the better design. If we want to have a test for this, I think it will likely be easier if we don't need to set up dummy files for the tests but can pass data in as variables. That would suggest that we use SQL to insert the data which in turn means parsing the files in TS.

const { stdout, stderr } = await execFile("sqlite3", [
DB,
".mode csv",
".import transports.csv transports",
".import muxers.csv muxers",
]);
if (stderr != "") {
throw new Error(`Could not parse csv resources: ${stderr}`);
}

const db = await open({
filename: DB,
driver: sqlite3.Database,
});

// Generate the testing combinations by SELECT'ing from both transports
// and muxers tables the distinct combinations where the transport and the muxer
// of the different libp2p implementations match.
const queryResults =
await db.all(`SELECT DISTINCT a.id as id1, b.id as id2, a.transport, ma.muxer
FROM transports a, transports b, muxers ma, muxers mb
WHERE a.id != b.id
AND a.transport == b.transport
AND a.id == ma.id
AND b.id == mb.id
AND ma.muxer == mb.muxer;`);
await db.close();

let global = new Global(
"multidimensional-testing",
"multidimensional",
argv.total_instances
);
let composition = new Composition(global, []);

for (let row of queryResults) {
// Instance count is hardcoded to 1 for now.
let instance = new Instance(1);

let test_params = new TestParams(row.transport, row.muxer);
let build_args1 = new BuildArgs(
row.id1,
argv.rustGitRef,
argv.rustGitTarget,
argv.goGitRef,
argv.goGitTarget
);

let build_config1 = new BuildConfig(build_args1);
let group1 = new Group(row.id1, BUILDER, instance, build_config1, test_params);

let build_args2 = new BuildArgs(
row.id2,
argv.rustGitRef,
argv.rustGitTarget,
argv.goGitRef,
argv.goGitTarget
);
let build_config2 = new BuildConfig(build_args2);
let group2 = new Group(row.id2, BUILDER, instance, build_config2, test_params);

let run = new Run(
`${row.id1} x ${row.id2} x ${row.transport} x ${row.muxer}`,
[group1, group2]
);

composition.runs.push(run);
}

// Write the TOML file and remove the database file to avoid corrupting
// future runs.
await writeFile("composition.toml", toml.stringify(composition as any));
await unlink(DB);
}

main()
.then(() => process.exit(0))
.catch((err) => {
console.error(err);
process.exit(1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

(another thread)

Wdyt of using just docker:generic to simplify the process @laurentsenta @mxinden?

If you want to tackle this question as part of this PR:

Initially, we planned on:

  • Implementing the test using testground builders + runners,
  • Rely on docker:generic and docker_extensions to implement missing features,
  • As we converge to a reliable solution, make the features Testground natives (add a docker:rust builder for example, and other simplifications).
  • With this approach, I would NOT recommend moving to docker:generic, but instead moving features into testground.

It seems we're now moving towards a different design:

});
Copy link
Collaborator

Choose a reason for hiding this comment

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

(creating here just to get a thread)

pass all the test parameters (transport, muxer, protocol) when launching the container the work becomes a lot easier for the composer and we could also combine it with testground/testground#1522.
Does testground currently support passing env vars when launching containers @laurentsenta?

Yes, the idiomatic solution is to use the run parameters,
there is an example of this in the ping test:

secureChannel = runenv.StringParam("secure_channel")
maxLatencyMs = runenv.IntParam("max_latency_ms")
iterations = runenv.IntParam("iterations")

You'd pass the parameters in the runs directly or in the runs.group field:

[runs.test_params]
iterations = "5"

11 changes: 11 additions & 0 deletions composer/muxers.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
id,muxer
Copy link
Member

Choose a reason for hiding this comment

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

Naive question, will there be an additional csv for security protocol TLS/Noise, or is that being handled differently?

Copy link
Member Author

Choose a reason for hiding this comment

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

will be another file, i.e. sec.csv:

id,protocol
rust-master,noise
rust-master, tls
rust-0.49.0,noise
rust-0.49.0,tls

that is then queried along the other ones.
The process of adding implementations can be improved later, either by creating a cli tool that does it or, getting another primary format, that is then parsed by the composer and converted to sql

rust-master,yamux
rust-master,mplex
rust-0.49.0,yamux
rust-0.49.0,mplex
rust-0.48.0,yamux
rust-0.48.0,mplex
rust-0.47.0,yamux
rust-0.47.0,mplex
go-0.23.4,yamux
go-0.23.4,mplex
Loading