Re: [PR] [GOBBLIN-2019] Remove circular dependency & initialize multi-active execution related classes [gobblin]

["umustafi (via GitHub)" <gi...@apache.org>]

umustafi commented on code in PR #3899:
URL: https://github.com/apache/gobblin/pull/3899#discussion_r1544756071


##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/core/GobblinServiceGuiceModule.java:
##########
@@ -202,21 +203,25 @@ public void configure(Binder binder) {
     OptionalBinder.newOptionalBinder(binder, DagManagementStateStore.class);
     OptionalBinder.newOptionalBinder(binder, DagProcFactory.class);
     OptionalBinder.newOptionalBinder(binder, DagProcessingEngine.class);
+    OptionalBinder.newOptionalBinder(binder, DagActionReminderScheduler.class);
     if (serviceConfig.isDagProcessingEngineEnabled()) {
-      binder.bind(DagManagement.class).to(DagManagementTaskStreamImpl.class);
-      binder.bind(DagTaskStream.class).to(DagManagementTaskStreamImpl.class);
-      binder.bind(DagManagementStateStore.class).to(MostlyMySqlDagManagementStateStore.class).in(Singleton.class);
-      binder.bind(DagProcFactory.class);
-      binder.bind(DagProcessingEngine.class);
-
+      /* Note that two instances of the same class can only be differentiated with an `annotatedWith` marker provided at
+      binding time (optionally bound classes cannot have names associated with them). Unlike, the scheduler lease arbiter,
+      the execution lease arbiter is used in single-active or multi-active execution. */
+      binder.bind(MultiActiveLeaseArbiter.class).
+              annotatedWith(Names.named(ConfigurationKeys.PROCESSING_LEASE_ARBITER_NAME))
+          .toProvider(
+              DagActionProcessingMultiActiveLeaseArbiterFactory.class);
       // Multi-active execution is only compatible with dagProcessingEngine configuration
-      OptionalBinder.newOptionalBinder(binder, ReminderSettingDagProcLeaseArbiter.class);
-      OptionalBinder.newOptionalBinder(binder, DagActionReminderScheduler.class);
       if (serviceConfig.isMultiActiveExecutionEnabled()) {
-        binder.bind(MultiActiveLeaseArbiter.class).annotatedWith(Names.named(EXECUTOR_LEASE_ARBITER_NAME)).to(MysqlMultiActiveLeaseArbiter.class);
         binder.bind(DagActionReminderScheduler.class);
-        binder.bind(ReminderSettingDagProcLeaseArbiter.class);
       }
+

Review Comment:
   Yes, I originally had it after but `DagManagementTaskStreamImpl` is dependent on `DagActionReminderScheduler`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org