ZihanLi58 commented on code in PR #3550:
URL: https://github.com/apache/gobblin/pull/3550#discussion_r980612114
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/MysqlUserQuotaManager.java:
##########
@@ -62,7 +80,7 @@ public void init(Collection<Dag<JobExecutionPlan>> dags) {
@Override
int incrementJobCount(String user, CountType countType) throws IOException {
try {
- return this.mysqlStore.increaseCount(user, countType);
+ return this.quotaStore.increaseCount(user, countType);
Review Comment:
Question here, shouldn't we change the increaseCount and addDagId to be one
mysql transaction? Otherwise we will see discrepancy between these two table?
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/flow/MultiHopFlowCompiler.java:
##########
@@ -281,6 +281,13 @@ public Dag<JobExecutionPlan> compileFlow(Spec spec) {
Instrumented.markMeter(flowCompilationSuccessFulMeter);
Instrumented.updateTimer(flowCompilationTimer, System.nanoTime() -
startTime, TimeUnit.NANOSECONDS);
+ if
(Boolean.parseBoolean(flowSpec.getConfigAsProperties().getProperty(ServiceConfigKeys.GOBBLIN_SERVICE_ADHOC_FLOW)))
{
+ // todo : we should probably set it on all of the start nodes
+ for (Dag.DagNode<JobExecutionPlan> dagNode :
jobExecutionPlanDag.getStartNodes()) {
Review Comment:
If we already change it to start nodes, should we remove the todo log on
line 285 also change the logic in compiler onAddFlowSpec to check quota for all
start nodes as well?
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/flow/MultiHopFlowCompiler.java:
##########
@@ -281,6 +281,13 @@ public Dag<JobExecutionPlan> compileFlow(Spec spec) {
Instrumented.markMeter(flowCompilationSuccessFulMeter);
Instrumented.updateTimer(flowCompilationTimer, System.nanoTime() -
startTime, TimeUnit.NANOSECONDS);
+ if
(Boolean.parseBoolean(flowSpec.getConfigAsProperties().getProperty(ServiceConfigKeys.GOBBLIN_SERVICE_ADHOC_FLOW)))
{
+ // todo : we should probably set it on all of the start nodes
+ for (Dag.DagNode<JobExecutionPlan> dagNode :
jobExecutionPlanDag.getStartNodes()) {
Review Comment:
Also if we have running dag Ids, do we still need this? anyway we should be
able to avoid double count using that map?
--
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]
