Skip to content

Commit

Permalink
queue policy: rename reconstruct into reconstruct_queue
Browse files Browse the repository at this point in the history
Problem: The semantics of reconstruct within the queue policy
implementation class is to reconstruct only the running
job queue.  The method name, reconstruct(), hence is
misleading as it may suggest the method would reconstruct
both the queue *and* resource state.

Change the name to be "reconstruct_queue" to clarify this.
Add more error checking code as part of this as well.
  • Loading branch information
dongahn committed Jun 11, 2020
1 parent 08ed4d3 commit 924341d
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 7 deletions.
2 changes: 1 addition & 1 deletion qmanager/policies/base/queue_policy_base.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ class queue_policy_base_impl_t
const std::shared_ptr<job_t> lookup (flux_jobid_t id);

protected:
int reconstruct (std::shared_ptr<job_t> running_job);
int reconstruct_queue (std::shared_ptr<job_t> running_job);
std::shared_ptr<job_t> pending_pop ();
std::shared_ptr<job_t> alloced_pop ();
std::shared_ptr<job_t> rejected_pop ();
Expand Down
29 changes: 23 additions & 6 deletions qmanager/policies/base/queue_policy_base_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ const std::shared_ptr<job_t> queue_policy_base_t::lookup (flux_jobid_t id)

int queue_policy_base_t::reconstruct (std::shared_ptr<job_t> running_job)
{
return detail::queue_policy_base_impl_t::reconstruct (running_job);
return detail::queue_policy_base_impl_t::reconstruct_queue (running_job);
}

std::shared_ptr<job_t> queue_policy_base_t::pending_pop ()
Expand Down Expand Up @@ -267,18 +267,35 @@ const std::shared_ptr<job_t> queue_policy_base_impl_t::lookup (flux_jobid_t id)
return m_jobs[id];
}

int queue_policy_base_impl_t::reconstruct (std::shared_ptr<job_t> job)
int queue_policy_base_impl_t::reconstruct_queue (std::shared_ptr<job_t> job)
{
int rc = -1;
std::pair<std::map<uint64_t, flux_jobid_t>::iterator, bool> ret;
std::pair<std::map<flux_jobid_t,
std::shared_ptr<job_t>>::iterator, bool> ret2;

if (job == nullptr || m_jobs.find (job->id) != m_jobs.end ()) {
errno = EINVAL;
goto out;
}
job->t_stamps.running_ts = m_rq_cnt++;
m_running.insert (std::pair<uint64_t, flux_jobid_t>(job->t_stamps.running_ts,
job->id));
m_jobs.insert (std::pair<flux_jobid_t, std::shared_ptr<job_t>> (job->id,
job));

ret = m_running.insert (std::pair<uint64_t, flux_jobid_t>(
job->t_stamps.running_ts, job->id));
if (ret.second == false) {
rc = -1;
errno = ENOMEM;
goto out;
}
ret2 = m_jobs.insert (std::pair<flux_jobid_t, std::shared_ptr<job_t>> (
job->id, job));
if (ret2.second == false) {
m_running.erase (ret.first);
rc = -1;
errno = ENOMEM;
goto out;
}

rc = 0;
out:
return rc;
Expand Down

0 comments on commit 924341d

Please sign in to comment.