phet commented on code in PR #3824:
URL: https://github.com/apache/gobblin/pull/3824#discussion_r1388754085
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/FlowTriggerHandler.java:
##########
@@ -129,6 +131,7 @@ public void handleTriggerEvent(Properties jobProps,
DagActionStore.DagAction flo
leaseObtainedStatus.getEventTimeMillis());
return;
}
+ this.failedPersistingLeaseCount.mark();
Review Comment:
shall we also have a meter for success? or are you planning to compute from
(successfullyObtained - failedToRecord/Persist)?
##########
gobblin-metrics-libs/gobblin-metrics/src/main/java/org/apache/gobblin/metrics/ServiceMetricNames.java:
##########
@@ -36,14 +36,15 @@ public class ServiceMetricNames {
public static final String FLOW_ORCHESTRATION_DELAY =
GOBBLIN_SERVICE_PREFIX_WITH_DELIMITER + "flowOrchestration.delay";
// Flow Trigger Handler
- public static final String FLOW_TRIGGER_HANDLER_PREFIX =
GOBBLIN_SERVICE_PREFIX_WITH_DELIMITER + "flowTriggerHandler";
- public static final String GOBBLIN_FLOW_TRIGGER_HANDLER_NUM_FLOWS_SUBMITTED
= FLOW_TRIGGER_HANDLER_PREFIX + ".numFlowsSubmitted";
- public static final String FLOW_TRIGGER_HANDLER_LEASE_OBTAINED_COUNT =
FLOW_TRIGGER_HANDLER_PREFIX + ".leaseObtained";
- public static final String FLOW_TRIGGER_HANDLER_LEASED_TO_ANOTHER_COUNT =
FLOW_TRIGGER_HANDLER_PREFIX + ".leasedToAnother";
- public static final String FLOW_TRIGGER_HANDLER_NO_LONGER_LEASING_COUNT =
FLOW_TRIGGER_HANDLER_PREFIX + ".noLongerLeasing";
- public static final String FLOW_TRIGGER_HANDLER_JOB_DOES_NOT_EXIST_COUNT =
FLOW_TRIGGER_HANDLER_PREFIX + ".jobDoesNotExistInScheduler";
- public static final String FLOW_TRIGGER_HANDLER_FAILED_TO_SET_REMINDER_COUNT
= FLOW_TRIGGER_HANDLER_PREFIX + ".failedToSetReminderCount";
- public static final String
FLOW_TRIGGER_HANDLER_LEASES_OBTAINED_DUE_TO_REMINDER_COUNT =
FLOW_TRIGGER_HANDLER_PREFIX + ".leasesObtainedDueToReminderCount";
+ public static final String FLOW_TRIGGER_HANDLER_PREFIX =
GOBBLIN_SERVICE_PREFIX_WITH_DELIMITER + "flowTriggerHandler.";
+ public static final String GOBBLIN_FLOW_TRIGGER_HANDLER_NUM_FLOWS_SUBMITTED
= FLOW_TRIGGER_HANDLER_PREFIX + "numFlowsSubmitted";
+ public static final String FLOW_TRIGGER_HANDLER_LEASE_OBTAINED_COUNT =
FLOW_TRIGGER_HANDLER_PREFIX + "leaseObtained";
+ public static final String FLOW_TRIGGER_HANDLER_LEASED_TO_ANOTHER_COUNT =
FLOW_TRIGGER_HANDLER_PREFIX + "leasedToAnother";
+ public static final String FLOW_TRIGGER_HANDLER_NO_LONGER_LEASING_COUNT =
FLOW_TRIGGER_HANDLER_PREFIX + "noLongerLeasing";
+ public static final String FLOW_TRIGGER_HANDLER_JOB_DOES_NOT_EXIST_COUNT =
FLOW_TRIGGER_HANDLER_PREFIX + "jobDoesNotExistInScheduler";
+ public static final String FLOW_TRIGGER_HANDLER_FAILED_TO_SET_REMINDER_COUNT
= FLOW_TRIGGER_HANDLER_PREFIX + "failedToSetReminderCount";
+ public static final String
FLOW_TRIGGER_HANDLER_LEASES_OBTAINED_DUE_TO_REMINDER_COUNT =
FLOW_TRIGGER_HANDLER_PREFIX + "leasesObtainedDueToReminderCount";
+ public static final String
FLOW_TRIGGER_HANDLER_FAILED_PERSISTING_LEASE_COUNT =
FLOW_TRIGGER_HANDLER_PREFIX + "failedPersistingLeaseCount";
Review Comment:
nit: the MALeaseArbiter API name is `recordLeaseSuccess`... seems more
consistent to title `failedToRecordLeaseSuccessCount`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
