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

Change audit orchestration and implement latest strategy #60

Merged
merged 1 commit into from
Mar 7, 2023

Conversation

perama-v
Copy link
Contributor

@perama-v perama-v commented Feb 11, 2023

Description

Updates the audit logic. This enables glados-audit to perform multiple audit strategies within the one binary.
The PR includes the logic for the latest strategy and stubs for additional strategies.

Closes #37.

Dependencies

Overview

Note: Glados-monitor is unaffected. It continues to watch the chain tip of a non-portal node
and push content keys into the glados database.

Multiple audit strategies are defined. Any (or all) may be invoked by spawning a new thread.
Each spawned thread sends audit tasks via a multi-producer single-consumer channel. Task-creating
threads do not alter the glados database.

An audit task is a content key from the the glados database that is expected to be in a portal node. Tasks are created according to different strategies:

  • latest implemented
  • random stub
  • failed stub
  • <additional unimplemented strategies may be added later>

A single thread watches the task channel and performs audits on anything received. Audit results
are stored in the glados database by this thread only, and no database locking is therefore required.

When glados-auditis started, all strategies are commenced. Any unimplemented strategy logs a warning.
A flag could later be added to chose a strategy/strategies. E.g.,

  • No flag: Use all strategies
  • Flag: Use comma-delimited value strategies only.

Changes

  • New selection module to handle the different strategies.
  • New SelectionStrategy enum

Usage

let strategy = SelectionStrategy::Latest;
tokio::spawn(strategy.select_latest_content_for_audit(tx.clone(), conn.clone()));

Testing

A test database generator is created that can be used for different strategies. See doctests for more.

Checklist

  • Add thread spawner for different strategies
  • Update latest with correct logic
  • Tidy commits
  • Add test for latest logic

Not addressed

The following may be completed in a different PR(s).

  • Add CLI
  • Implement failed strategy
  • Implement random strategy

@perama-v perama-v marked this pull request as draft February 11, 2023 08:37
@perama-v perama-v changed the title [WIP] Change audit strategy Change audit strategy Feb 12, 2023
@perama-v perama-v marked this pull request as ready for review February 12, 2023 04:25
Copy link
Collaborator

@mrferris mrferris left a comment

Choose a reason for hiding this comment

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

Looks good!

Only thing I'd change are a few naming suggestions. The main one is that I think the name selection.rs describes what's being done here better than generator.rs.

One suggestion for a future PR is that we include within the audit's DB record which selection strategy it was triggered by.

glados-audit/src/generator.rs Outdated Show resolved Hide resolved
glados-audit/src/generator.rs Outdated Show resolved Hide resolved
glados-audit/src/generator.rs Outdated Show resolved Hide resolved
glados-audit/src/generator.rs Outdated Show resolved Hide resolved
glados-audit/src/generator.rs Outdated Show resolved Hide resolved
glados-audit/src/generator.rs Outdated Show resolved Hide resolved
glados-audit/src/generator.rs Outdated Show resolved Hide resolved
@perama-v
Copy link
Contributor Author

One suggestion for a future PR is that we include within the audit's DB record which selection strategy it was triggered by.

See #63

@perama-v perama-v marked this pull request as ready for review February 15, 2023 02:45
@perama-v
Copy link
Contributor Author

All suggested changes have been made

@pipermerriam
Copy link
Member

can you rebase now that #59 is merged.

@perama-v
Copy link
Contributor Author

Rebased on master

@pipermerriam
Copy link
Member

has conflicts now... probably from the other one I merged.

I'll get this merged and deployed tomorrow if the conflicts can get resolved by then. Thanks again for all the great work.

@perama-v
Copy link
Contributor Author

perama-v commented Feb 16, 2023

Rebase on master done 👍

@perama-v perama-v changed the title Change audit strategy Change audit orchestration and implement latest strategy Feb 22, 2023
@perama-v perama-v marked this pull request as draft February 22, 2023 23:36
@perama-v
Copy link
Contributor Author

Testing of the latest strategy is now included.

@perama-v perama-v marked this pull request as ready for review February 23, 2023 06:28
@pipermerriam pipermerriam merged commit d6b5845 into ethereum:master Mar 7, 2023
@perama-v perama-v deleted the change-audit-strategy branch March 16, 2023 05:17
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.

Change audit strategy
3 participants