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

update roadmap #56

Merged
merged 6 commits into from
Oct 20, 2022
Merged

update roadmap #56

merged 6 commits into from
Oct 20, 2022

Conversation

p-shahi
Copy link
Member

@p-shahi p-shahi commented Oct 18, 2022

This PR

  • updates the roadmap with multi-dimension test/interoperability efforts
  • pulls in testing and benchmarking from libp2p implementation roadmaps
  • organizes in style of the go and js documents

TODO:

  • if the projects defined are agreed upon, create Epics/issues for items
  • assign completion best guess

@p-shahi p-shahi marked this pull request as ready for review October 19, 2022 01:00
Comment on lines -28 to -29
- We fixed every known stability issue with the `libp2p/test-plans` test suite
- [Issue 36](https://github.com/libp2p/test-plans/issues/36)
Copy link
Member Author

Choose a reason for hiding this comment

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

I chose to remove this as it's ongoing maintenance work

Comment on lines -59 to -60
- `libp2p/interop` and `libp2p/test-plans` are working together
- They are either merged or somehow know about each other.
Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't tell the difference between this goal vs

We have ported the tests from libpp2/interop

So I chose to subsume it under "Be the home for all interop tests"

Comment on lines +180 to +183
<!-- TODO: Assign a quarter -->
<!-- TODO: Create issue -->
**Why:** Having interoperability tests with lots of transports, encryption mechanisms, and stream muxers is great. However, we need to stay backwards-compatible with legacy libp2p releases, with other libp2p implementations, and less advanced libp2p stacks.
Copy link
Member Author

Choose a reason for hiding this comment

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

This is listed last here but likely merits being much higher up. Rough idea is to itemize our test backlog (across implementations), prioritize, and then commit to chipping away at it over the 2023 H1 and H2.

Comment on lines +170 to +173
**Why**: For libp2p to be competitive, it needs to delivers comparable performance to widely used protocols on the internet, namely HTTP/2 and HTTP/3.

**Goal**: We have a test suite that runs libp2p transfers between nodes located at different locations all over the world, proving that libp2p is able to achieve performance on par with HTTP. The test suite is run on a continuous basis and results are published to a public performance dashboard.
Copy link
Member Author

Choose a reason for hiding this comment

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

Deliverables for both milestones are in Testground, but idea is that it lives here so we can use test-plans and multi-dimension test framework to measure transfer performance for all implementations.

ROADMAP.md Show resolved Hide resolved
@@ -58,6 +61,11 @@ This document describes our process for testing interoperability & backward comp

# How do we test libp2p interop at ProtocolLabs?

---

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is basically the multi-dimension tests (semantics and process)

Copy link
Contributor

@BigLep BigLep left a comment

Choose a reason for hiding this comment

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

Feel free to merge from my perspective if comments are incorporated.

ROADMAP.md Outdated Show resolved Hide resolved
ROADMAP.md Outdated Show resolved Hide resolved
ROADMAP.md Show resolved Hide resolved
ROADMAP.md Show resolved Hide resolved
ROADMAP.md Outdated Show resolved Hide resolved
ROADMAP.md Outdated Show resolved Hide resolved
ROADMAP.md Outdated
#### 3. Rock solid libp2p releases
<!-- TODO: Create issue -->
The full `libp2p/test-plans` test suite is run before releasing a version of go-libp2p, rust-libp2p, and js-libp2p.
Copy link
Contributor

@BigLep BigLep Oct 19, 2022

Choose a reason for hiding this comment

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

I think we should run the interoperability testplan today (or at least soon) before releases. It's fine if it's manual.

Can we pull this portion in sooner?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think run the interoperability testplan should occur today (or at least soon) before releases.

I'm having trouble parsing this sentence.

It's fine if it's manual.

The problem is that we have many parts of our stack that are currently untested (other than unit tests): hole punching, autonat, autorelay etc. We manually test these parts when we make changes to the respective code paths, but we don't rerun these manual tests on every release. It's just too much stuff to test.
This is not ideal, and we should have automated interoperability tests for all these components. Requiring manual testing though would add hours of testing effort to our release process.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we all agree that these tests are needed like yesterday. But we don’t have this yet, so what’s the fastest way we can get to the world where we have this? I’m afraid manual smoke testing will cut into the effort to build this.

I’m also afraid that adding more friction to the release process will cause patches to be unnecessarily delayed.

If we weren’t prioritizing these projects, I would agree that we should at least have a manual test checklist. But since we are, it seems to me we can live with the status quo for a bit longer if it gets us this faster.

Copy link
Contributor

Choose a reason for hiding this comment

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

@marten-seemann @MarcoPolo : My bad on incoherent sentence. Fixed now.

Correct me if this is wrong, but my understanding is that we have some degree of interoperability testing run as part of CI but there is also a "big test" that doesn't run as part of CI because it takes too long.

What I'm suggesting we do today in our release process is run the big test manually as one of the release checklist items.

I'm not suggesting we do any manual smoke testing features that don't have a test-plan (unless of course we made changes in one of those areas). I'm suggesting we do make sure we execute the test-plan we have written that is flexing interoperability.

In future, I expect the "big interoperability test" either gets run as part of CI (because we have a cluster than can execute it quickly enough) or it runs as a nightly cron job and the release process is one of verifying that last night's run passed.

Does that make sense?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed "3. Rock solid libp2p releases" and combined with "2. Interop test plans for all existing/developing libp2p transports"

3's intention was to run the transport interop tests as CI nightly job, serving as regression tests for new release candidates.

ROADMAP.md Outdated
A GitHub workflow is added so that these suites run for any PR against master and run against the `master` branch on a nightly basis (updating the status check.)

#### 4. Canonical interop tests & dashboard
Copy link
Contributor

@BigLep BigLep Oct 19, 2022

Choose a reason for hiding this comment

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

I would vote to do this sooner - maybe as item 2. It gives a very clear signal to the community of what we're aiming for and how far we have to go. It also becomes a clear way for us to show progress each month. I'd love at monthly all hands for us to show the month over month change of this.

Copy link
Contributor

@MarcoPolo MarcoPolo Oct 19, 2022

Choose a reason for hiding this comment

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

Isn’t point 2 a prereq for this issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

@MarcoPolo : I don't think it's a prerequ. I'm viewing this as analogous to other behaviours in software develop;meant like putting in the metric before you make the performance improvement or writing the test before the implementation. The dashboard makes clear what we're aiming for. It's our baseline. To start with it will be red/empty for all/most the combinations. Then as we do A.2 "Interop test plans for all existing/developing features" we'll start showing the matrix progressively filling more-and-more each week.

I think this came up in a verbal conversation with @mxinden as well (which is making me think it would be good to use sync time in Lisbon to be on the same page here), but I'm not suggesting to prioritize the dashboard so we can claim interoperability or pad ourselves on the back. I want so we have a clear target that we can show steady progress against.

Does that make sense?

Copy link
Member Author

@p-shahi p-shahi Oct 20, 2022

Choose a reason for hiding this comment

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

To start with it will be red/empty for all/most the combinations.

IIUC, I think this should still be possible with A.1's simple dashboard, but it'll be a markdown output and require manual effort to: trigger to run the interop test plans and generate/update the resulting dashboard.

The comprehensiveness of the simple dashboard would be dependent on the permutations specified in the resource/composition file. From what I understand, I think we can specify all permutations as soon as the tooling is ready. It should also be possible to list incompatible/unimplemented combinations in the resource file (the testground run will fail and we can display the failure how we like.)

It also becomes a clear way for us to show progress each month

Having said that I think A.1 can meet this goal.
From what I understood, it really depends on the inputs provided and massaging the output csv file to markdown.

A.3 as I intended to describe it is: once all interop test-plans are written and running in CI, we can focus on generating a nicer output that can be shared on a website. An output that is updated from the CI run without manual intervention; maybe even hosted on interoperability.libp2p.io

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm good to be scrappy here. At the minimum I want to make sure we can produce/show the interoperability dashboard with its dimensionality monthly during our all-hands (and ideally highlight the deltas). Even better (but not required) would be if this was a primary "metric" reported by libp2p in the weekly sitrep. I'm good with however we get there. If it's a major manual effort then it won't happen, so I'm good to invest in automation early here. Manually starting a github action on a monthly basis to generate the data seems good enough to me to start. Thanks all.

ROADMAP.md Outdated Show resolved Hide resolved
ROADMAP.md Outdated
<!-- TODO: Create issue -->
We have ported the tests from `libp2p/interop`
- This repository implement tests `connect`, `dht`, `pubsub`, `steam` ([ref](https://github.com/libp2p/interop/blob/ce0aa3749c9c53cf5ad53009b273847b94106d40/src/index.ts#L32-L35))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- This repository implement tests `connect`, `dht`, `pubsub`, `steam` ([ref](https://github.com/libp2p/interop/blob/ce0aa3749c9c53cf5ad53009b273847b94106d40/src/index.ts#L32-L35))
- This repository implement tests `connect`, `dht`, `pubsub` ([ref](https://github.com/libp2p/interop/blob/ce0aa3749c9c53cf5ad53009b273847b94106d40/src/index.ts#L32-L35))

It's stream, not steam. This test just seems to be a simple echo handler, so this should be handled by our ping test already.

ROADMAP.md Outdated
<!-- TODO: Create issue -->
We have a more stable build process that doesn't risk breaking
- We generate artifacts for old versions during merges to the libp2p repositories https://github.com/libp2p/test-plans/issues/35#issuecomment-1254991985
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if a stable build process deserves its own point. This seems like the bare minimum that's needed for testing anyway.

Generating artifacts, if I understand correctly, would just be an optimization to speed up build times.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not 100% sure whether to keep or remove this.
@laurentsenta do you have any suggestions?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@marten-seemann has a point, no strong feelings against removing these

ROADMAP.md Outdated Show resolved Hide resolved
ROADMAP.md Outdated Show resolved Hide resolved
ROADMAP.md Outdated Show resolved Hide resolved
@p-shahi
Copy link
Member Author

p-shahi commented Oct 20, 2022

Discussed in person with Laurent, will be merging this into his branch no.w

@p-shahi p-shahi merged commit f8cf56b into feat/add-design-doc Oct 20, 2022
@p-shahi p-shahi deleted the roadmap-updates branch October 21, 2022 06:40
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.

5 participants