Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store RRDP data in one file per repository. #886

Merged
merged 23 commits into from
Nov 29, 2023
Merged

Store RRDP data in one file per repository. #886

merged 23 commits into from
Nov 29, 2023

Conversation

partim
Copy link
Member

@partim partim commented Aug 18, 2023

This PR implements a very simple archive that can be used to store, update, and access the objects published by an RRDP repository in a single file.

@partim partim marked this pull request as draft August 18, 2023 15:14
Copy link
Contributor

@timbru timbru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By and large looks good to me. I have (non-blocking) questions:

  • would you consider keeping a growable index table to avoid sequential reads for find
  • would you consider putting this in its own tiny crate so that it could be used by the kvx crate as well?

src/utils/archive.rs Show resolved Hide resolved
@partim
Copy link
Member Author

partim commented Aug 21, 2023

Regarding re-using it in kvx, this has been designed specifically for RRDP, not sure if it would also fit more generic use-cases. For instances, it can’t deal very well with growing objects. Not a problem with RRDP, but might be with other uses.

@timbru
Copy link
Contributor

timbru commented Aug 21, 2023

Regarding re-using it in kvx, this has been designed specifically for RRDP, not sure if it would also fit more generic use-cases. For instances, it can’t deal very well with growing objects. Not a problem with RRDP, but might be with other uses.

Yeah, let's have that discussion face-to-face then. Generally speaking kvx could be used for growing objects. For Krill specifically we're using the key-value store largely as an append-only store for new changes (although we also store snapshots). Saving those changes as separate JSON files is kind of useful for debugging, but it can result in a large number of files and inodes being used on busy systems.

@partim partim marked this pull request as ready for review September 25, 2023 10:15
Copy link
Member

@density215 density215 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few remarks/suggestions.

src/collector/rrdp/archive.rs Show resolved Hide resolved
src/utils/archive.rs Show resolved Hide resolved
src/utils/archive.rs Show resolved Hide resolved
src/utils/archive.rs Show resolved Hide resolved
src/utils/archive.rs Show resolved Hide resolved
src/engine.rs Show resolved Hide resolved
@partim partim removed the request for review from DRiKE November 29, 2023 16:25
@partim partim merged commit c47461c into main Nov 29, 2023
10 checks passed
@partim partim deleted the files-in-file branch November 29, 2023 16:26
partim added a commit that referenced this pull request Jun 10, 2024
Breaking changes

* Keep the content of an RRDP repository in a single file rather than
  as individual files under a directory. ([#886])
* Changed the `summary` output format to have all lines end in a
  semicolon. ([#907])
* Changed the options used for `rsync`. The options `-rtO --delete` are
  now always used. The options set in the `rsync-args` are added or, if
  that is not used, `-z` and `--no-motd`, as well as `--contimeout=10`
  if it is supported by the rsync command, and `--max-size` if the
  `max-object-size` option has not been set to 0. ([#962])

New

* The `chain_validity` value in the `jsonext` format now considers the
  validity of the manifest’s EE certificates. A new `stale` value shows
  the time when any of the publication points along the way will become
  stale. ([#945])
* If a collected manifest has a lower manifest number or an older
  thisUpdate field than a stored manifest for the same CA, the collected
  manifest is ignored and the stored publication point is used instead.
  This implements a requirement added in [RFC 9286]. ([#946], [#954])
* The number of delta entries in a RRDP notification file is now limited
  to 500 by default. If there are more entries, the deltas are ignored and
  the snapshot is used. The limit can be changed through the new
  `rrdp-max-delta-list-len` configuration value. ([#961])
* The RRDP collector now falls back to a snapshot update if the hash of
  a delta listed in the notification file has changed from the previous
  update. This implements [draft-ietf-sidrops-rrdp-desynchronization-00].
  ([#951])
* The RRDP collector now enforces that all URIs referred to or redirected
  to by an RRDP server have the same origin as the rpkiNotify URI in the
  CA certificate. ([#953])
* The config file used is now printed for some commands. This should help
  with avoiding confusion when running Routinator as different users.
  ([#959])

Bug fixes

* Fixed an issue where the refresh time was calculated as zero under
  certain conditions until the dataset was updated. ([#940])
* Add the current RRDP serial number to the RRDP server metrics when a
  Not Modified response is received so that Prometheus shows a constant
  value.
partim added a commit that referenced this pull request Jun 20, 2024
…970)

Breaking changes

* Keep the content of an RRDP repository in a single file rather than
  as individual files under a directory. ([#886])
* Switched to the all-new version 0.4 of the Routinator UI. This also
  changes the way we import the UI into Routinator by simply including the
  built assets which means downloads are not necessary during the build
  process any more. ([#952])
* Changed the `summary` output format to have all lines end in a
  semicolon. ([#907])
* Changed the options used for `rsync`. The options `-rtO --delete` are
  now always used. The options set in the `rsync-args` are added or, if
  that is not used, `-z` and `--no-motd`, as well as `--contimeout=10`
  if it is supported by the rsync command, and `--max-size` if the
  `max-object-size` option has not been set to 0. ([#962])

New

* The `chain_validity` value in the `jsonext` format now considers the
  validity of the manifest’s EE certificates. A new `stale` value shows
  the time when any of the publication points along the way will become
  stale. ([#945])
* If a collected manifest has a lower manifest number or an older
  thisUpdate field than a stored manifest for the same CA, the collected
  manifest is ignored and the stored publication point is used instead.
  This implements a requirement added in [RFC 9286]. ([#946], [#954])
* The number of delta entries in a RRDP notification file is now limited
  to 500 by default. If there are more entries, the deltas are ignored and
  the snapshot is used. The limit can be changed through the new
  `rrdp-max-delta-list-len` configuration value. ([#961])
* The RRDP collector now falls back to a snapshot update if the hash of
  a delta listed in the notification file has changed from the previous
  update. This implements [draft-ietf-sidrops-rrdp-desynchronization-00].
  ([#951])
* The RRDP collector now enforces that all URIs referred to or redirected
  to by an RRDP server have the same origin as the rpkiNotify URI in the
  CA certificate. ([#953])
* The config file used is now printed for some commands. This should help
  with avoiding confusion when running Routinator as different users.
  ([#959])

Bug fixes

* Fixed an issue where the refresh time was calculated as zero under
  certain conditions until the dataset was updated. ([#940])
* Add the current RRDP serial number to the RRDP server metrics when a
  Not Modified response is received so that Prometheus shows a constant
  value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants