-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[Feature][Core] Add PendingJobs info in the response of GetOverviewOperation #9902
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
[Feature][Core] Add PendingJobs info in the response of GetOverviewOperation #9902
Conversation
overviewInfo.setPendingJobs( | ||
server.getCoordinatorService().getJobCountMetrics().getPendingJobCount()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about creating a method in CoordinatorService
that directly returns only the pendingJobCount instead of using the getJobCountMetrics
method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use
Line 51 in b5f583b
private final Map<Long, E> jobIdMap = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- the constructor method of the JobHistoryService
jobHistoryService =
new JobHistoryService(
nodeEngine,
runningJobStateIMap,
logger,
pendingJobQueue.getJobIdMap(),
runningJobMasterMap,
nodeEngine.getHazelcastInstance().getMap(Constant.IMAP_FINISHED_JOB_STATE),
nodeEngine
.getHazelcastInstance()
.getMap(Constant.IMAP_FINISHED_JOB_METRICS),
nodeEngine
.getHazelcastInstance()
.getMap(Constant.IMAP_FINISHED_JOB_VERTEX_INFO),
engineConfig.getHistoryJobExpireMinutes());
- the getJobStatusData method of JobHistoryService
public List<JobStatusData> getJobStatusData() {
List<JobStatusData> status = new ArrayList<>();
final List<JobState> runningJobStateList =
runningJobMasterMap.values().stream()
.map(master -> toJobStateMapper(master, true))
.collect(Collectors.toList());
Set<Long> runningJonIds =
runningJobStateList.stream().map(JobState::getJobId).collect(Collectors.toSet());
List<JobState> pendingJobStateList =
pendingJobInfoMap.entrySet().stream()
.map(
entry -> {
Long jobId = entry.getKey();
JobImmutableInformation jobImmutableInformation =
entry.getValue()
.getJobMaster()
.getJobImmutableInformation();
return new JobState(
jobId,
jobImmutableInformation.getJobName(),
JobStatus.PENDING,
jobImmutableInformation.getCreateTime(),
null,
null,
null,
null);
})
.collect(Collectors.toList());
....
jobIdMap has been assigned to pendingJobInfoMap
of the JobHistoryService
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion, the getJobStatusData()
and getJobCountMetrics()
methods perform a lot of extra work that isn’t needed if we only want to get the pendingJobCount
.
Purpose of this pull request
Add PendingJobs info in the response of GetOverviewOperation
Does this PR introduce any user-facing change?
How was this patch tested?
Check list
New License Guide