1k-validators-be icon indicating copy to clipboard operation
1k-validators-be copied to clipboard

Scorekeeper Refactoring 1/4

Open vovacha opened this issue 1 year ago • 2 comments

This pull request initiates the refactoring of the Scorekeeper and Gateway components. The goal of this refactoring is to simplify the codebase, reduce technical debt, and improve maintainability and scalability. This is the first stage of a multi-step process to achieve a cleaner and more modular architecture.

Problems:

  • Locality Issues. The Job class has several injections, such as RegisterHandler and cron/StartCronJobs/startJob, causing locality issues.
  • Complexity. There is excessive complexity with too many abstraction layers, including MonolithJobRunner, MicroserviceJobRunner, JobsRunnerFactory, JobsRunner, JobFactory, and individual classes for each job.
  • Dead Code. There is commented-out code, unused event handlers in Scorekeeper (only job events are used), and MicroserviceJobRunner is particularly problematic.
  • Tight Coupling with Gateway. The Scorekeeper directly feeds the Gateway with job statuses.

Final goal of the Scorekeeper refactoring:

  • Simplify Abstractions. Reduce all abstractions to a single Job class. The state will be represented by the DB layer only.
  • Unified Flow. Ensure a consistent flow from JobConfig to Job class instance to job.start().
  • Organize Specific Jobs. Maintain specific jobs in separate locations, similar to the current structure, with minor changes.
  • Gateway Independence. Ensure the Gateway consumes data only from the DB, eliminating its dependency on the Scorekeeper.

Current PR changes:

  • scorekeeper.ts. Minor changes. A new entry point and syntax improvements.
  • JobConfigs.ts. Simplified the job initialization process. The JobNames enum will be removed in a future update.
  • types.ts. New file. Contains existing types related to the Job class.
  • Job.ts. New file. Contains only the Job class, supports the old public interfaces, and explicitly shows dependency injection for the startJobFunction, which will be removed later.
  • specificJobs. Made minor changes to create a common interface by supporting the metadata parameter, even if it's not used.
  • RegisterHandlers.ts. Removed dead code and moved job events to the Job class.
  • scorekeeper.int.test.ts. Added an integration test to check the job status update on event emission.
  • JobsClass.ts. Deleted. This file exists temporarily to minimize the impact of the current PR and will be removed in a future update.
  • MonolithJobRunner.ts, MicroserviceJobRunner.ts, JobsRunnerFactory.ts, JobRunner.ts, JobFactory.ts. Deleted.

Potential next steps (to be discussed):

  • [ ] Scorekeeper 1. Current PR.
  • [ ] Scorekeeper 2. Localize behavior by bringing cron job initialization into the Job class. Remove JobNames enum. https://github.com/w3f/1k-validators-be/pull/2937
  • [ ] Scorekeeper 3. Add DB queries to store job status.
  • [ ] Gateway. Update views to retrieve job statuses from the DB.
  • [ ] Gateway. Remove Scorekeeper dependency.
  • [ ] Core. Update Gateway initialization without Scorekeeper.
  • [ ] Scorekeeper 4. Remove status updates via events and eliminate public interfaces related to the Gateway.
  • [ ] Gateway. Reduce abstraction layers and code duplication in controllers.
  • [ ] Gateway. Refactor dynamic routes (e.g., onlyHealth flag).
  • [ ] Deployment Update. Update deployment scripts to run services separately: Scorekeeper, Gateway, and Telemetry.

vovacha avatar May 17 '24 13:05 vovacha