Page MenuHomeSoftware Heritage
Differential D7477

Improve `revision_get_parents` implementation in `ArchiveGraph`
AbandonedPublic
Actions

Authored by douardda on Mar 31 2022, 12:18 PM.

Details

Reviewers
vlorentz
aeviso
Group Reviewers
Reviewers
Summary

When a revision is passed, it now retrieves and caches the complete
history three to minimize the queries to the graph.

Fix get_archive to avoid storage parameter for ArchiveGraph

Diff Detail

Event Timeline

aeviso created this revision.Mar 31 2022, 12:18 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptMar 31 2022, 12:18 PM
swh-public-ci added a comment.Mar 31 2022, 2:29 PM

Build is green

Patch application report for D7477 (id=27129)

Rebasing onto 16723f8be4...

Current branch diff-target is up to date.
Changes applied before test
commit 67d7c5733e9b4648b3fb07918b69107fc6bcd036
Author: Andres Ezequiel Viso <aeviso@softwareheritage.org>
Date:   Thu Mar 31 12:16:19 2022 +0200

    Fix `get_archive` to avoid `storage` parameter for `ArchiveGraph`

commit 9f77e41049f268ae388f0a1b77cfa7a0c3b49e32
Author: Andres Ezequiel Viso <aeviso@softwareheritage.org>
Date:   Thu Mar 31 11:42:47 2022 +0200

    Improve `revision_get_parents` implementation in `ArchiveGraph`
    
    When a revision is passed, it now retrieves and caches the complete
    history three to minimize the queries to the graph.

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

Harbormaster completed remote builds in B28046: Diff 27129.Mar 31 2022, 2:29 PM
aeviso requested review of this revision.Mar 31 2022, 2:29 PM
swh-public-ci added a comment.Mar 31 2022, 2:30 PM

Build is green

Patch application report for D7477 (id=27129)

Rebasing onto 16723f8be4...

Current branch diff-target is up to date.
Changes applied before test
commit 67d7c5733e9b4648b3fb07918b69107fc6bcd036
Author: Andres Ezequiel Viso <aeviso@softwareheritage.org>
Date:   Thu Mar 31 12:16:19 2022 +0200

    Fix `get_archive` to avoid `storage` parameter for `ArchiveGraph`

commit 9f77e41049f268ae388f0a1b77cfa7a0c3b49e32
Author: Andres Ezequiel Viso <aeviso@softwareheritage.org>
Date:   Thu Mar 31 11:42:47 2022 +0200

    Improve `revision_get_parents` implementation in `ArchiveGraph`
    
    When a revision is passed, it now retrieves and caches the complete
    history three to minimize the queries to the graph.

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

aeviso added a child revision: D7478: Clean up `model` removing old unused logic.Mar 31 2022, 3:25 PM
vlorentz accepted this revision.Apr 5 2022, 3:58 PM
This revision is now accepted and ready to land.Apr 5 2022, 3:58 PM
douardda commandeered this revision.Oct 13 2022, 4:38 PM
douardda abandoned this revision.
douardda added a reviewer: aeviso.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
provenance/
3 lines50%0%
swhgraph/
27 lines96%100%
tests/
29 lines84%100%

Diff 27129

View Options

swh/provenance/__init__.py

Show All 38 Lines elif cls == "direct":
from .postgresql.archive import ArchivePostgreSQL from .postgresql.archive import ArchivePostgreSQL
return ArchivePostgreSQL(BaseDb.connect(**kwargs["db"]).conn) return ArchivePostgreSQL(BaseDb.connect(**kwargs["db"]).conn)
elif cls == "graph": elif cls == "graph":
try: try:
from swh.graph.client import RemoteGraphClient from swh.graph.client import RemoteGraphClient
from swh.storage import get_storage
from .swhgraph.archive import ArchiveGraph from .swhgraph.archive import ArchiveGraph
graph = RemoteGraphClient(kwargs.get("url")) graph = RemoteGraphClient(kwargs.get("url"))
return ArchiveGraph(graph, get_storage(**kwargs["storage"])) return ArchiveGraph(graph, get_storage(cls="memory"))
except ModuleNotFoundError: except ModuleNotFoundError:
raise EnvironmentError( raise EnvironmentError(
"Graph configuration required but module is not installed." "Graph configuration required but module is not installed."
) )
else: else:
raise ValueError raise ValueError
▲ Show 20 Lines • Show All 59 Lines • Show Last 20 Lines
View Options

swh/provenance/swhgraph/archive.py

# Copyright (C) 2022 The Software Heritage developers # Copyright (C) 2022 The Software Heritage developers
# See the AUTHORS file at the top-level directory of this distribution # See the AUTHORS file at the top-level directory of this distribution
# License: GNU General Public License version 3, or any later version # License: GNU General Public License version 3, or any later version
# See top-level LICENSE file for more information # See top-level LICENSE file for more information
from typing import Any, Dict, Iterable from typing import Any, Dict, Iterable, Set
from swh.core.statsd import statsd from swh.core.statsd import statsd
from swh.model.model import Sha1Git from swh.model.model import Sha1Git
from swh.model.swhids import CoreSWHID, ObjectType from swh.model.swhids import CoreSWHID, ObjectType
from swh.storage.interface import StorageInterface from swh.storage.interface import StorageInterface
ARCHIVE_DURATION_METRIC = "swh_provenance_archive_graph_duration_seconds" ARCHIVE_DURATION_METRIC = "swh_provenance_archive_graph_duration_seconds"
class ArchiveGraph: class ArchiveGraph:
def __init__(self, graph, storage: StorageInterface) -> None: def __init__(self, graph, storage: StorageInterface) -> None:
self.graph = graph self.graph = graph
self.storage = storage # required by ArchiveInterface self.storage = storage # required by ArchiveInterface
self.parents: Dict[Sha1Git, Set[Sha1Git]] = {}
@statsd.timed(metric=ARCHIVE_DURATION_METRIC, tags={"method": "directory_ls"}) @statsd.timed(metric=ARCHIVE_DURATION_METRIC, tags={"method": "directory_ls"})
def directory_ls(self, id: Sha1Git, minsize: int = 0) -> Iterable[Dict[str, Any]]: def directory_ls(self, id: Sha1Git, minsize: int = 0) -> Iterable[Dict[str, Any]]:
raise NotImplementedError raise NotImplementedError
@statsd.timed( @statsd.timed(
metric=ARCHIVE_DURATION_METRIC, tags={"method": "revision_get_parents"} metric=ARCHIVE_DURATION_METRIC, tags={"method": "revision_get_parents"}
) )
def revision_get_parents(self, id: Sha1Git) -> Iterable[Sha1Git]: def revision_get_parents(self, id: Sha1Git) -> Iterable[Sha1Git]:
if id not in self.parents:
self.parents = {}
src = CoreSWHID(object_type=ObjectType.REVISION, object_id=id) src = CoreSWHID(object_type=ObjectType.REVISION, object_id=id)
request = self.graph.neighbors(str(src), edges="rev:rev", return_types="rev") edges = {
(
CoreSWHID.from_string(child).object_id,
CoreSWHID.from_string(parent).object_id,
)
for child, parent in self.graph.visit_edges(str(src), edges="rev:rev")
}
for child, parent in edges:
self.parents.setdefault(child, set()).add(parent)
self.parents.setdefault(parent, set())
yield from (CoreSWHID.from_string(swhid).object_id for swhid in request) yield from self.parents[id]
@statsd.timed(metric=ARCHIVE_DURATION_METRIC, tags={"method": "snapshot_get_heads"}) @statsd.timed(metric=ARCHIVE_DURATION_METRIC, tags={"method": "snapshot_get_heads"})
def snapshot_get_heads(self, id: Sha1Git) -> Iterable[Sha1Git]: def snapshot_get_heads(self, id: Sha1Git) -> Iterable[Sha1Git]:
src = CoreSWHID(object_type=ObjectType.SNAPSHOT, object_id=id) src = CoreSWHID(object_type=ObjectType.SNAPSHOT, object_id=id)
request = self.graph.visit_nodes( request = self.graph.visit_nodes(
str(src), edges="snp:rev,snp:rel,rel:rev", return_types="rev" str(src), edges="snp:rev,snp:rel,rel:rev", return_types="rev"
) )
yield from (CoreSWHID.from_string(swhid).object_id for swhid in request) yield from (
CoreSWHID.from_string(swhid).object_id for swhid in request if swhid
)
View Options

swh/provenance/tests/test_archive_interface.py

Show All 25 Linesfrom swh.model.model import (
SnapshotBranch, SnapshotBranch,
TargetType, TargetType,
) )
from swh.model.swhids import CoreSWHID, ExtendedObjectType, ExtendedSWHID from swh.model.swhids import CoreSWHID, ExtendedObjectType, ExtendedSWHID
from swh.provenance.archive import ArchiveInterface from swh.provenance.archive import ArchiveInterface
from swh.provenance.postgresql.archive import ArchivePostgreSQL from swh.provenance.postgresql.archive import ArchivePostgreSQL
from swh.provenance.storage.archive import ArchiveStorage from swh.provenance.storage.archive import ArchiveStorage
from swh.provenance.swhgraph.archive import ArchiveGraph from swh.provenance.swhgraph.archive import ArchiveGraph
from swh.provenance.tests.conftest import fill_storage, load_repo_data from swh.provenance.tests.conftest import fill_storage, load_repo_data, objs_from_dict
from swh.storage.postgresql.storage import Storage from swh.storage.postgresql.storage import Storage
def check_directory_ls( def check_directory_ls(
reference: ArchiveInterface, archive: ArchiveInterface, data: Dict[str, List[dict]] reference: ArchiveInterface, archive: ArchiveInterface, data: Dict[str, List[dict]]
) -> None: ) -> None:
for directory in data["directory"]: for directory in data["directory"]:
entries_ref = sorted( entries_ref = sorted(
Show All 40 Lines elif object_type == "directory":
return Directory return Directory
elif object_type == "revision": elif object_type == "revision":
return Revision return Revision
elif object_type == "snapshot": elif object_type == "snapshot":
return Snapshot return Snapshot
raise ValueError raise ValueError
def data_to_model(data: Dict[str, List[dict]]) -> Dict[str, List[BaseModel]]:
model: Dict[str, List[BaseModel]] = {}
for object_type, objects in data.items():
for object in objects:
model.setdefault(object_type, []).append(
get_object_class(object_type).from_dict(object)
)
return model
def add_link( def add_link(
edges: Set[ edges: Set[
Tuple[ Tuple[
Union[CoreSWHID, ExtendedSWHID, str], Union[CoreSWHID, ExtendedSWHID, str] Union[CoreSWHID, ExtendedSWHID, str], Union[CoreSWHID, ExtendedSWHID, str]
] ]
], ],
src_obj: Union[Origin, Snapshot, Revision, Directory, Content], src_obj: Union[Origin, Snapshot, Revision, Directory, Content],
dst_id: bytes, dst_id: bytes,
Show All 15 Lines
]: ]:
nodes: Set[Union[CoreSWHID, ExtendedSWHID, str]] = set() nodes: Set[Union[CoreSWHID, ExtendedSWHID, str]] = set()
edges: Set[ edges: Set[
Tuple[ Tuple[
Union[CoreSWHID, ExtendedSWHID, str], Union[CoreSWHID, ExtendedSWHID, str] Union[CoreSWHID, ExtendedSWHID, str], Union[CoreSWHID, ExtendedSWHID, str]
] ]
] = set() ] = set()
model = data_to_model(data) objects = {
objtype: [objs_from_dict(objtype, d) for d in dicts]
for objtype, dicts in data.items()
}
for origin in model["origin"]: for origin in objects["origin"]:
assert isinstance(origin, Origin) assert isinstance(origin, Origin)
nodes.add(origin.swhid()) nodes.add(origin.swhid())
for status in model["origin_visit_status"]: for status in objects["origin_visit_status"]:
assert isinstance(status, OriginVisitStatus) assert isinstance(status, OriginVisitStatus)
if status.origin == origin.url and status.snapshot is not None: if status.origin == origin.url and status.snapshot is not None:
add_link(edges, origin, status.snapshot, ExtendedObjectType.SNAPSHOT) add_link(edges, origin, status.snapshot, ExtendedObjectType.SNAPSHOT)
for snapshot in model["snapshot"]: for snapshot in objects["snapshot"]:
assert isinstance(snapshot, Snapshot) assert isinstance(snapshot, Snapshot)
nodes.add(snapshot.swhid()) nodes.add(snapshot.swhid())
for branch in snapshot.branches.values(): for branch in snapshot.branches.values():
assert isinstance(branch, SnapshotBranch) assert isinstance(branch, SnapshotBranch)
if branch.target_type in [TargetType.RELEASE, TargetType.REVISION]: if branch.target_type in [TargetType.RELEASE, TargetType.REVISION]:
target_type = ( target_type = (
ExtendedObjectType.RELEASE ExtendedObjectType.RELEASE
if branch.target_type == TargetType.RELEASE if branch.target_type == TargetType.RELEASE
else ExtendedObjectType.REVISION else ExtendedObjectType.REVISION
) )
add_link(edges, snapshot, branch.target, target_type) add_link(edges, snapshot, branch.target, target_type)
for revision in model["revision"]: for revision in objects["revision"]:
assert isinstance(revision, Revision) assert isinstance(revision, Revision)
nodes.add(revision.swhid()) nodes.add(revision.swhid())
# root directory # root directory
add_link(edges, revision, revision.directory, ExtendedObjectType.DIRECTORY) add_link(edges, revision, revision.directory, ExtendedObjectType.DIRECTORY)
# parent # parent
for parent in revision.parents: for parent in revision.parents:
add_link(edges, revision, parent, ExtendedObjectType.REVISION) add_link(edges, revision, parent, ExtendedObjectType.REVISION)
for directory in model["directory"]: for directory in objects["directory"]:
assert isinstance(directory, Directory) assert isinstance(directory, Directory)
nodes.add(directory.swhid()) nodes.add(directory.swhid())
for entry in directory.entries: for entry in directory.entries:
assert isinstance(entry, DirectoryEntry) assert isinstance(entry, DirectoryEntry)
if entry.type == "file": if entry.type == "file":
target_type = ExtendedObjectType.CONTENT target_type = ExtendedObjectType.CONTENT
elif entry.type == "dir": elif entry.type == "dir":
target_type = ExtendedObjectType.DIRECTORY target_type = ExtendedObjectType.DIRECTORY
elif entry.type == "rev": elif entry.type == "rev":
target_type = ExtendedObjectType.REVISION target_type = ExtendedObjectType.REVISION
add_link(edges, directory, entry.target, target_type) add_link(edges, directory, entry.target, target_type)
for content in model["content"]: for content in objects["content"]:
assert isinstance(content, Content) assert isinstance(content, Content)
nodes.add(content.swhid()) nodes.add(content.swhid())
return list(nodes), list(edges) return list(nodes), list(edges)
@pytest.mark.parametrize( @pytest.mark.parametrize(
"repo", "repo",
Show All 31 Lines
About · Developer information · Legal