feat: pullstorage iterator share

[janos]

This is an alternative approach to share pullstorage iterators by using resenje.org/singleflight which is context aware compared to the x/sync/singleflight implementation. The relevant functionality is described here https://pkg.go.dev/resenje.org/singleflight#Group.Do.

Currently, this is just a POC, unit tests should be added before merging.

---

This change is 

[aloknerurkar]

Can we add a test here for IntervalChunks function? I think you can use the first test I have for IteratorShare. Although there seem to be enough tests in the lib to verify, it will be good to have one in this package.

[mrekucci]

Reviewed 3 of 3 files at r1, 2 of 2 files at r2, 1 of 1 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @acud, @esadakar, and @janos)

---

pkg/pullsync/pullstorage/pullstorage_test.go, line 313 at r2 (raw file):

		// delay is needed in order to have the iterator
		// linger for a bit longer for more chunks.
		<-time.After(200 * time.Millisecond)

Using time.Sleep would be more efficient.

---

pkg/pullsync/pullstorage/pullstorage_test.go, line 356 at r2 (raw file):

	}

	// check that results point to same array

Examine the internal is dangerous since you're relying on assumptions that might not be true in the feature. I'd recommend comparing the content of these arrays directly; if you want to save some lines I'd recommend using: https://github.com/google/go-cmp for that (It checks for Equal method, not like reflect.DeepEqual).

---

pkg/pullsync/pullstorage/pullstorage_test.go, line 436 at r2 (raw file):

				}
				// check that results point to same array
				sh := (*reflect.SliceHeader)(unsafe.Pointer(&res.addrs))

The same as above.

[aloknerurkar]

---

pkg/pullsync/pullstorage/pullstorage_test.go, line 356 at r2 (raw file):

Previously, mrekucci (Peter Mrekaj) wrote…

Examine the internal is dangerous since you're relying on assumptions that might not be true in the feature. I'd recommend comparing the content of these arrays directly; if you want to save some lines I'd recommend using: https://github.com/google/go-cmp for that (It checks for Equal method, not like reflect.DeepEqual).

This verifies the feature. If we get multiple similar requests for IntervalChunks we should get a shared result. This result is used for readOnly.

[mrekucci]

Reviewed 1 of 1 files at r4.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @acud and @esadakar)

---

pkg/pullsync/pullstorage/pullstorage_test.go, line 356 at r2 (raw file):

Previously, aloknerurkar (Alok Nerurkar) wrote…

This verifies the feature. If we get multiple similar requests for IntervalChunks we should get a shared result. This result is used for readOnly.

I'm concern that the values wouldn't be the same. I don't know whether this shared iterator should also prevent doing that. Because we're using mutable addresses and if one goroutine shares a slice with another even they are pointing to the same top slice, because of the mutable nature, the content in time can be different.

[mrekucci]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @acud and @esadakar)