[GitHub] [sling-org-apache-sling-event] stefan-egli commented on a change in pull request #16: SLING-11091: expose the number of scheduled jobs as metric

[GitBox <gi...@apache.org>]

stefan-egli commented on a change in pull request #16:
URL: https://github.com/apache/sling-org-apache-sling-event/pull/16#discussion_r800572442



##########
File path: src/main/java/org/apache/sling/event/impl/jobs/scheduling/JobSchedulerImpl.java
##########
@@ -455,6 +455,14 @@ private boolean match(final ScheduledJobInfoImpl job, final Map<String, Object>
         }
         return jobs;
     }
+    
+    /**
+     * Provide the total number of jobs registered in the system, irrespective of topics
+     * @return the total number of scheduled jobs
+     */
+    public int getTotalNumberOfScheduledJobs() {
+       return this.scheduledJobs.size(); 

Review comment:
               synchronized ( this.scheduledJobs ) {
               return this.scheduledJobs.size();
           }

##########
File path: src/main/java/org/apache/sling/event/impl/jobs/JobManagerImpl.java
##########
@@ -91,6 +95,8 @@
     })
 public class JobManagerImpl
     implements JobManager, EventHandler, Runnable {
+    
+    private static final String GAUGE_TOTAL_SCHEDULED_JOBS = "totalScheduledJobs";

Review comment:
       regarding the naming here, I was wondering if we should prefix similar to the [GAUGE_PREFIX](https://github.com/apache/sling-org-apache-sling-event/blob/master/src/main/java/org/apache/sling/event/impl/jobs/stats/GaugeSupport.java#L35) - or maybe a sibling `"event.scheduledjobs"` to avoid any chance of naming conflict?

##########
File path: src/main/java/org/apache/sling/event/impl/jobs/scheduling/JobSchedulerImpl.java
##########
@@ -455,6 +455,14 @@ private boolean match(final ScheduledJobInfoImpl job, final Map<String, Object>
         }
         return jobs;
     }
+    
+    /**
+     * Provide the total number of jobs registered in the system, irrespective of topics
+     * @return the total number of scheduled jobs
+     */
+    public int getTotalNumberOfScheduledJobs() {
+       return this.scheduledJobs.size(); 

Review comment:
       I think the above needs synchronized protection. Which is somewhat of a performance hit, but I think that's reasonable (unless we'd rewrite it to be a ConcurrentHashMap)




-- 
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@sling.apache.org

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