pubsub makes no guarantee on Windows and with go-ipfs

[richardschneider]

Skip some tests because pubsub, go floodsub, does not make any guarantee on message delivery.

First reported in ipfs-inactive/js-ipfs-http-client#648

1) .pubsub multiple nodes connected multiple nodes receive multiple messages:
     Error: Timeout of 80000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

2) .pubsub multiple nodes connected load tests send/receive 10k messages:
     Error: Timeout of 80000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

[daviddias]

@richardschneider this looks like an incorrect assessment. The tests have shown only that go-ipfs misses delivering messages to itself (having the same node be the publisher and the subscriber) only on Windows which tells us that there might be some real issue and not just a protocol design. There have been more users that reported the same.

[richardschneider]

Took a look at go-floodsub and this code looks suspicious

binary.BigEndian.PutUint64(seqno, uint64(time.Now().UnixNano()))

Duplicate sequence number can be produced

• if the resolution of the clock is not nanoseconds, or
• mutliple publish events are received within the resolution of the clock

[daviddias]

@richardschneider added a check to see if the test is being run with go-ipfs. However, we should also check if it is Windows given that the issue only happens there.

We can't use the os module as it wouldn't work in the Browser. Open to ideas :)

[mkg20001]

@diasdavid Maybe aegir could set an environment variable like AEGIR_OS which could be included in the browser.

[richardschneider]

@diasdavid The issue was not really Windows related but was related to the resolution of the system clock; see issue libp2p/go-libp2p-pubsub#52.

The fix is in the pipe-line. So it should be resolved in the next go release.

Let's merge this but raise an issue to re-test when next go release is available.

[daviddias]

@Stebalien ETA for the next go-ipfs release?

[richardschneider]

@diasdavid @Stebalien still waiting on the next go release, any estimate?

[Stebalien]

"soon", unfortunately, this is a very big release. We need to get ipfs/kubo#4610 merged first, then test it, and then fix any fallout.

[daviddias]

@richardschneider let's do a skip on Windows + go-ipfs for these tests and add a // TODO to follow up after release

[richardschneider]

@diasdavid Code to skip if go-ipfs and TODO are already in place. This is not a windows issue.

RTM

[vmx]

Tested with js-ipfs (Linux) and js-ipfs-api (Linux, Windows).

[victorb]

const isWindows = process.platform && process.platform === 'win32'

Doesn't standard warn you here that this is a uneccessary conditional?

[vmx]

Good catch. I didn't get a warning but fixed it nonetheless.