Paths

Table of Contentst

Differential D7838

Add an overlay storage
Needs RevisionPublic
Actions

Authored by vlorentz on May 16 2022, 7:54 PM.

Details

Reviewers

douardda

Group Reviewers

Reviewers

Summary

I will use it to locally test incremental loads of large repositories
which are already loaded in production, with read-only access of the
production storage.

This may also be used in the future to access storages while
migrating/replicating.

Test Plan

This is fully tested when the backend storages are empty (by inheriting the usual tests);
and never tested when they are not (for now)

I also tried loading linux.git with this config:

storage:
  cls: pipeline
  steps:
    - cls: buffer
    - cls: filter
    - cls: overlay
      storages:
        - cls: memory
        - cls: remote
          url: http://webapp.internal.staging.swh.network:5002/

and it didn't use more than 4GB of RAM! (and my machined almost OOMed without the filter proxy), so it seems to work

Diff Detail

Repository

rDSTO Storage manager

Branch

master

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 29417
Build 45974: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 45973: arc lint + arc unit

Event Timeline

vlorentz created this revision.May 16 2022, 7:54 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptMay 16 2022, 7:54 PM

Harbormaster returned this revision to the author for changes because remote builds failed.May 16 2022, 7:54 PM

Harbormaster failed remote builds in B29416: Diff 28314!

update docstring

vlorentz edited the test plan for this revision. (Show Details)May 16 2022, 7:56 PM

Build is green

Patch application report for D7838 (id=28314)

Rebasing onto cb12394c34...

Current branch diff-target is up to date.

Changes applied before test

commit cc02746dcbd0b96769befc231e16e7b2497d5def
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon May 16 19:52:29 2022 +0200

    Add an overlay storage
    
    I will use it to locally test incremental loads of large repositories
    which are already loaded in production, with read-only access of the
    production storage.
    
    This may also be used in the future to access storages while
    migrating/replicating.

See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1611/ for more details.

Build is green

Patch application report for D7838 (id=28315)

Rebasing onto cb12394c34...

Current branch diff-target is up to date.

Changes applied before test

commit 1760e539ea47de6f6a9f6a311aded4d564ca22d7
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon May 16 19:52:29 2022 +0200

    Add an overlay storage
    
    I will use it to locally test incremental loads of large repositories
    which are already loaded in production, with read-only access of the
    production storage.
    
    This may also be used in the future to access storages while
    migrating/replicating.

See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1612/ for more details.

Harbormaster completed remote builds in B29417: Diff 28315.May 16 2022, 8:08 PM

vlorentz requested review of this revision.May 16 2022, 8:08 PM

vlorentz edited the test plan for this revision. (Show Details)May 16 2022, 8:16 PM

add missing endpoints

Build is green

Patch application report for D7838 (id=28328)

Rebasing onto cb12394c34...

Current branch diff-target is up to date.

Changes applied before test

commit b606d636da8d4bfa446da408bacff6f4ea8bdd25
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon May 16 19:52:29 2022 +0200

    Add an overlay storage
    
    I will use it to locally test incremental loads of large repositories
    which are already loaded in production, with read-only access of the
    production storage.
    
    This may also be used in the future to access storages while
    migrating/replicating.

See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/1613/ for more details.

Harbormaster completed remote builds in B29432: Diff 28328.May 17 2022, 3:26 PM

douardda added a subscriber: douardda.Jun 1 2022, 12:39 PM

douardda added inline comments.

swh/storage/proxies/overlay.py
51	wouldn't it make sense to give this a clear and specific argument name rather then being "the first of the 'storages' list"? maybe something like: storage: cls: overlay rw_storage: cls: remote url: xxx ro_storages: - cls: remote url: xxx - cls: remote url: xxx
60	counter?

douardda added inline comments.Jun 1 2022, 1:25 PM

swh/storage/proxies/overlay.py
137	weird sentence, looks like a piece is missing.

douardda added inline comments.Jun 1 2022, 1:35 PM

swh/storage/proxies/overlay.py
83	why is this (specifically) needed?
135	not sure I would not prefer something without this Big Bad getattr. eg. class OverlayProxyStorage: content_get_data = self._getter_optional("content_get_data") directory_get_entries = self._getter_optional("directory_get_entries") ... probably not worth the effort however...

(not a surprise but) it really needs at least one test of the "overlay feature" for each method...

swh/storage/tests/storage_tests.py
563	what do we lose with these unsorted comparisions? can this be a problem?

douardda requested changes to this revision.Jun 1 2022, 1:40 PM

This revision now requires changes to proceed.Jun 1 2022, 1:40 PM

vlorentz added inline comments.Jun 1 2022, 1:53 PM

swh/storage/proxies/overlay.py
83	because other proxies have a `storage` attribute that is sometimes expected; and I don't want this to raise `NotImplementedError`
135	I'm not sure methods would be bound if we do it this way, so it would be more complicated
swh/storage/tests/storage_tests.py
563	nothing, this was already not guaranteed, and only worked in tests accidentally