Page MenuHomeSoftware Heritage
Differential D1635

indexing-lister: Allow to define flush packet size
ClosedPublic
Actions

Authored by ardumont on Jun 23 2019, 9:35 AM.

Details

Reviewers
olasd
douardda
vlorentz
Group Reviewers
Reviewers
Commits
rDLS6662ae8db5f1: indexing_lister: Allow to define flush packet size
Summary

Prior to this commit, indexing lister instances were flushing every packet of

  1. This can now be defined per sub classes.

For the bitbucket lister, as the number of repositories grew from 10 per page to
100 per page, that enlarged the time frame between flushes.

Depends on D1638

Test Plan

tox

Diff Detail

Repository
rDLS Listers
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Jun 23 2019, 9:35 AM
ardumont edited the summary of this revision. (Show Details)Jun 23 2019, 9:36 AM
ardumont added projects: Lister, Bitbucket lister.
ardumont added a parent revision: D1634: lister: Type correctly the 'indexable' column.
Harbormaster failed remote builds in B6429: Diff 5441!Jun 23 2019, 9:39 AM
swh-public-ci added a comment.Jun 23 2019, 9:39 AM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLS/job/tox/229/
See console output for more information: https://jenkins.softwareheritage.org/job/DLS/job/tox/229/console

swh-public-ci added a comment.Jun 23 2019, 9:40 AM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLS/job/tox/231/
See console output for more information: https://jenkins.softwareheritage.org/job/DLS/job/tox/231/console

ardumont updated this revision to Diff 5443.Jun 23 2019, 9:53 AM

Rebase on latest D1634's commit content

ardumont updated this revision to Diff 5445.EditedJun 23 2019, 9:56 AM

Rebase on latest D1634

Harbormaster failed remote builds in B6431: Diff 5443!Jun 23 2019, 9:58 AM
swh-public-ci added a comment.Jun 23 2019, 9:58 AM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLS/job/tox/233/
See console output for more information: https://jenkins.softwareheritage.org/job/DLS/job/tox/233/console

Harbormaster failed remote builds in B6433: Diff 5445!Jun 23 2019, 9:59 AM
swh-public-ci added a comment.Jun 23 2019, 9:59 AM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLS/job/tox/235/
See console output for more information: https://jenkins.softwareheritage.org/job/DLS/job/tox/235/console

ardumont planned changes to this revision.Jun 24 2019, 4:33 PM
ardumont updated this revision to Diff 5467.Jun 24 2019, 5:49 PM

Update to latest code

ardumont retitled this revision from indexing-lister: Allow to define flush packet db size to indexing-lister: Allow to define flush packet size.Jun 24 2019, 5:49 PM
Harbormaster failed remote builds in B6457: Diff 5467!Jun 24 2019, 5:51 PM
swh-public-ci added a comment.Jun 24 2019, 5:51 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLS/job/tox/246/
See console output for more information: https://jenkins.softwareheritage.org/job/DLS/job/tox/246/console

ardumont added a child revision: D1638: lister: Clean up code.Jun 24 2019, 6:07 PM
ardumont updated this revision to Diff 5475.Jun 25 2019, 1:13 PM

Update to latest commit

Harbormaster failed remote builds in B6465: Diff 5475!Jun 25 2019, 1:14 PM
swh-public-ci added a comment.Jun 25 2019, 1:14 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLS/job/tox/250/
See console output for more information: https://jenkins.softwareheritage.org/job/DLS/job/tox/250/console

ardumont updated this revision to Diff 5482.Jun 25 2019, 2:31 PM

Rebase

Harbormaster failed remote builds in B6472: Diff 5482!Jun 25 2019, 2:33 PM
ardumont removed a child revision: D1638: lister: Clean up code.Jun 25 2019, 2:33 PM
swh-public-ci added a comment.Jun 25 2019, 2:33 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLS/job/tox/258/
See console output for more information: https://jenkins.softwareheritage.org/job/DLS/job/tox/258/console

ardumont edited the summary of this revision. (Show Details)Jun 25 2019, 2:33 PM
ardumont added a parent revision: D1638: lister: Clean up code.
swh-public-ci added a comment.Jun 25 2019, 2:34 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLS/job/tox/259/
See console output for more information: https://jenkins.softwareheritage.org/job/DLS/job/tox/259/console

ardumont added a child revision: D1632: bitbucket: Allow incremental lister to start properly the first time.Jun 25 2019, 2:36 PM
ardumont removed a parent revision: D1634: lister: Type correctly the 'indexable' column.
swh-public-ci added a comment.Jun 25 2019, 2:49 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLS/job/tox/263/
See console output for more information: https://jenkins.softwareheritage.org/job/DLS/job/tox/263/console

ardumont updated this revision to Diff 5487.Jun 25 2019, 2:57 PM

Rebase

swh-public-ci added a comment.Jun 25 2019, 3:05 PM

Build is green
See https://jenkins.softwareheritage.org/job/DLS/job/tox/267/ for more details.

Harbormaster completed remote builds in B6477: Diff 5487.Jun 25 2019, 3:05 PM
vlorentz requested changes to this revision.Jun 26 2019, 10:24 AM
vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
swh/lister/bitbucket/lister.py
32

I'm confused by this comment. Prior behavior of what? (I can deduce IndexingLister because it's in the same diff, but it won't make sense afterward.) And why does the Bitbucket lister need to override this behavior?

This revision now requires changes to proceed.Jun 26 2019, 10:24 AM
ardumont added inline comments.Jun 26 2019, 10:28 AM
swh/lister/bitbucket/lister.py
32

Because i changed the packet size returned by the api from 10 repositories (too small) to 100 repositories (a tad better) for the bitbucket listing.

so 2 iterations of 100 repositories, i already have the 200 repositories to flush in db.
If i kept the original indexing lister, i would have changed the behavior to flush every 2000 repositories.

vlorentz accepted this revision.Jun 26 2019, 10:33 AM
vlorentz added inline comments.
swh/lister/core/indexing_lister.py
18

This argument should have a short docstring

This revision is now accepted and ready to land.Jun 26 2019, 10:33 AM
Closed by commit rDLS6662ae8db5f1: indexing_lister: Allow to define flush packet size (authored by ardumont). · Explain WhyJun 26 2019, 11:19 AM
This revision was automatically updated to reflect the committed changes.
ardumont mentioned this in D1643: indexing_lister: Add docstrings to flush_packet_db & default_min_bound.Jun 26 2019, 11:29 AM
ardumont mentioned this in rDLSe54531510c91: indexing_lister: Add docstrings to flush_packet_db & default_min_bound.Jun 26 2019, 12:37 PM

Revision Contents
Changeset List

PathSize
swh/
lister/
bitbucket/
3 lines
core/
5 lines

Diff 5502

View Options

swh/lister/bitbucket/lister.py

Show All 23 Linesclass BitBucketLister(IndexingHttpLister):
LISTER_NAME = 'bitbucket' LISTER_NAME = 'bitbucket'
instance = 'bitbucket' instance = 'bitbucket'
def __init__(self, api_baseurl, override_config=None, per_page=100): def __init__(self, api_baseurl, override_config=None, per_page=100):
super().__init__( super().__init__(
api_baseurl=api_baseurl, override_config=override_config) api_baseurl=api_baseurl, override_config=override_config)
if per_page != DEFAULT_BITBUCKET_PAGE: if per_page != DEFAULT_BITBUCKET_PAGE:
self.PATH_TEMPLATE = '%s&pagelen=%s' % ( self.PATH_TEMPLATE = '%s&pagelen=%s' % (
self.PATH_TEMPLATE, per_page) self.PATH_TEMPLATE, per_page)
vlorentzUnsubmitted
Not Done
Inline Actions

I'm confused by this comment. Prior behavior of what? (I can deduce IndexingLister because it's in the same diff, but it won't make sense afterward.) And why does the Bitbucket lister need to override this behavior?

vlorentz: I'm confused by this comment. Prior behavior of what? (I can deduce IndexingLister because it's…
ardumontAuthorUnsubmitted
Done
Inline Actions

Because i changed the packet size returned by the api from 10 repositories (too small) to 100 repositories (a tad better) for the bitbucket listing.

so 2 iterations of 100 repositories, i already have the 200 repositories to flush in db.
If i kept the original indexing lister, i would have changed the behavior to flush every 2000 repositories.

ardumont: Because i changed the packet size returned by the api from 10 repositories (too small) to 100…
# to stay consistent with prior behavior (20 * 10 repositories then)
self.flush_packet_db = int(
(self.flush_packet_db * DEFAULT_BITBUCKET_PAGE) / per_page)
def get_model_from_repo(self, repo): def get_model_from_repo(self, repo):
return { return {
'uid': repo['uuid'], 'uid': repo['uuid'],
'indexable': repo['created_on'], 'indexable': repo['created_on'],
'name': repo['name'], 'name': repo['name'],
'full_name': repo['full_name'], 'full_name': repo['full_name'],
'html_url': repo['links']['html']['href'], 'html_url': repo['links']['html']['href'],
▲ Show 20 Lines • Show All 55 Lines • Show Last 20 Lines
View Options

swh/lister/core/indexing_lister.py

# Copyright (C) 2015-2017 the Software Heritage developers # Copyright (C) 2015-2019 the Software Heritage developers
# 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
import abc import abc
import logging import logging
from itertools import count from itertools import count
import dateutil import dateutil
from sqlalchemy import func from sqlalchemy import func
from .lister_transports import ListerHttpTransport from .lister_transports import ListerHttpTransport
from .lister_base import ListerBase from .lister_base import ListerBase
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
class IndexingLister(ListerBase): class IndexingLister(ListerBase):
vlorentzUnsubmitted
Not Done
Inline Actions

This argument should have a short docstring

vlorentz: This argument should have a short docstring
flush_packet_db = 20
"""Lister* intermediate class for any service that follows the pattern: """Lister* intermediate class for any service that follows the pattern:
- The service must report at least one stable unique identifier, known - The service must report at least one stable unique identifier, known
herein as the UID value, for every listed repository. herein as the UID value, for every listed repository.
- If the service splits the list of repositories into sublists, it must - If the service splits the list of repositories into sublists, it must
report at least one stable and sorted index identifier for every listed report at least one stable and sorted index identifier for every listed
repository, known herein as the indexable value, which can be used as repository, known herein as the indexable value, which can be used as
part of the service endpoint query to request a sublist beginning from part of the service endpoint query to request a sublist beginning from
▲ Show 20 Lines • Show All 190 Lines • ▼ Show 20 Lines def run(self, min_bound=None, max_bound=None):
logger.info('stopping after index %s, no next link found', logger.info('stopping after index %s, no next link found',
index) index)
return return
index = next_index index = next_index
logger.debug('Index: %s', index) logger.debug('Index: %s', index)
yield i yield i
for i in ingest_indexes(): for i in ingest_indexes():
if (i % 20) == 0: if (i % self.flush_packet_db) == 0:
logger.debug('Flushing updates at index %s', i) logger.debug('Flushing updates at index %s', i)
self.db_session.commit() self.db_session.commit()
self.db_session = self.mk_session() self.db_session = self.mk_session()
self.db_session.commit() self.db_session.commit()
self.db_session = self.mk_session() self.db_session = self.mk_session()
class IndexingHttpLister(ListerHttpTransport, IndexingLister): class IndexingHttpLister(ListerHttpTransport, IndexingLister):
"""Convenience class for ensuring right lookup and init order """Convenience class for ensuring right lookup and init order
when combining IndexingLister and ListerHttpTransport.""" when combining IndexingLister and ListerHttpTransport."""
def __init__(self, api_baseurl=None, override_config=None): def __init__(self, api_baseurl=None, override_config=None):
ListerHttpTransport.__init__(self, api_baseurl=api_baseurl) ListerHttpTransport.__init__(self, api_baseurl=api_baseurl)
IndexingLister.__init__(self, override_config=override_config) IndexingLister.__init__(self, override_config=override_config)
About · Developer information · Legal