I'mt not exactly sure why I thought that would be necessary for benchmarking. In any case... it's not ;-)

The documentation is at:

It's documented in the winery test environment and was actually able to use the instructions successfully (after a few fixes...). It does work an this can be closed as resolved.

Added a wiki page to be a more accessible version of the benchmark process than the README in the sources.

Adjust the disk names on grid5000 to reflect changes at https://www.grid5000.fr/w/Grenoble:Hardware#dahu

In D7033#182832, @olasd wrote:

Thanks! (and sorry for the hash algo ping-pong)

address review comments

remove spurious print

In D7026#182606, @vlorentz wrote:

Could you add a short readme or something in winery-test-environment/mitogen-strategy/, so future reader know the rationale and where this is coming from?

• Added a README.txt to explain the need for the mitogen plugins
• Added mitogen as a test dependency

ensure the SSH key is 600 otherwise it leads to non-intuitive failures

• Remove spurious table added to sharedbase
• Reduce the number of PG in the Ceph cluster: 100 is enough for test purposes

Looks good, as usually.

This patch contains:

https://github.com/mjpieters/aiolimiter is a better fit and may be used for copy/pasting

https://github.com/vutran1710/PyrateLimiter looks really good but is not for IO throttling

https://www.django-rest-framework.org/api-guide/throttling/ is already used by swh-web/swh/web/api/throttling.py but is targeting REST API throttling, not a good fit.

rebase on top of master

Nevermind. I just realized that https://forge.softwareheritage.org/D6796 was in the "Accepted" state and has been for almost two weeks. :facepalm:

Gentle ping?

Happy new year ! Or, in French, "Bonne année deux mille vingt d'oeufs" (pun on words courtesy Easter-Eggs).

Hum, it looks like I forgot to submit two replies a week ago or so, doing that now.

Gentle ping?

I updated D6796 to add a high level description of the files, classes and logic. I also split the WineryObjStorage class into four classes in an attempt to clarify the separation between the read and the write logic. I will now refrain from doing anything else: it can be difficult on the reviewers to follow a moving target :-D

• Update misleading comment in the check function

• add implementation notes to the documentation

• Reworked the Winery class hierarchy to separate reader and writer

s/LOGGER/logger/

is actually necessary for jenkins to be happy. Not sure why it passes on my machine, that needs to be investigated.

@olasd @vlorentz @vsellier @ardumont I'd be happy to start polishing the implementation after your first wave of review (thanks a lot for the time you spent on this, it is much appreciated 👍 ). I'm however concerned that it might make your work difficult because of the magnitude of the patch. An alternative would be to merge this first implementation, as long as you feel confident it won't break anything when deployed. It should not because it won't be instantiated. That would allow me to propose patches that are smaller and easier to review for:

• address olasd & vlorentz comments

On the code, I've left a few inline comments.

@douardda will you have time for reviewing this week? I stand ready to address your comments. I'll stop working on that for now so that it is not a moving target.

• implement benchmarking

• avoid race condition when two processes try to create a table

In D6834#177663, @vlorentz wrote:

Ah wait I got it, docker/services/swh-winery/entrypoint.sh launches winery itself, not an actual objstorage backend; you're only reusing the scafholding.

rebase

In D6835#177644, @ardumont wrote:

And thanks ;)

In D6834#177606, @vlorentz wrote:

Instead of defining a new service, could you provide an alternative docker-compose config file? This way, it can be used as to switch all services to use it, just by adding a CLI parameter. eg. we do this to replace the postgres storage backend with cassandra: https://docs.softwareheritage.org/devel/getting-started/using-docker.html#cassandra

In D6835#177617, @ardumont wrote:

lgtm

iirc, you can replace all your __ by _ at the end of the link.

remove extra space added by mistake

It depends on https://forge.softwareheritage.org/D6796 and will fail until it is merged. It can be tested from sources with an override like this: