Skip to content
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.

Sign up for GitHub

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

RQ2 minimal functionality and bug fix to reapi match allocate #1101

Merged
merged 6 commits into from
Dec 14, 2023
Merged

RQ2 minimal functionality and bug fix to reapi match allocate #1101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e66a3da
reapi: track match success and update info for match allocate
zekemorton Oct 9, 2023
c19ddaf
reapi: add overloaded info function to reapi_cli_t
zekemorton Oct 9, 2023
e8e9862
reapi: add accessors for traverser pre/post order count
zekemorton Oct 10, 2023
6c7ca2e
rq: rq2 structure with match allocate
zekemorton Oct 10, 2023
922b6dc
tests: add tests for rq2
zekemorton Oct 11, 2023
e4f265d
tests: update golang tests and output
zekemorton Oct 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions resource/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ schema/test/schema_test01
schema/test/schema_test02
utilities/grug2dot
utilities/resource-query
utilities/rq2
6 changes: 6 additions & 0 deletions resource/reapi/bindings/c++/reapi_cli.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ class resource_query_t {
const std::shared_ptr<job_info_t> &get_job (const uint64_t jobid);
const bool reservation_exists (const uint64_t jobid);
const bool allocation_exists (const uint64_t jobid);
const unsigned int preorder_count ();
const unsigned int postorder_count ();

/* Mutators */
void clear_resource_query_err_msg ();
Expand Down Expand Up @@ -141,6 +143,10 @@ class reapi_cli_t : public reapi_t {
static int find (void *h, std::string criteria, json_t *&o );
zekemorton marked this conversation as resolved.
Show resolved Hide resolved
static int info (void *h, const uint64_t jobid, std::string &mode,
bool &reserved, int64_t &at, double &ov);
static int info (void *h, const uint64_t jobid,
std::shared_ptr<job_info_t> &job);
static unsigned int preorder_count (void *h);
static unsigned int postorder_count (void *h);
static int stat (void *h, int64_t &V, int64_t &E,int64_t &J,
double &load, double &min, double &max, double &avg);
static const std::string &get_err_message ();
Expand Down
100 changes: 78 additions & 22 deletions resource/reapi/bindings/c++/reapi_cli_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
std::shared_ptr<job_info_t> job_info = nullptr;
struct timeval start_time, end_time;
std::stringstream o;
bool matched = false;

try {
Flux::Jobspec::Jobspec job {jobspec};
Expand Down Expand Up @@ -99,6 +100,19 @@
rc = -1;
zekemorton marked this conversation as resolved.
Show resolved Hide resolved
goto out;
}

if ( (rc != 0) && (errno == ENOMEM)) {
m_err_msg += __FUNCTION__;
m_err_msg += ": ERROR: Memory error for "
+ std::to_string (rq->get_job_counter ());
rc = -1;
goto out;

Check warning on line 109 in resource/reapi/bindings/c++/reapi_cli_impl.hpp

View check run for this annotation

Codecov / codecov/patch

resource/reapi/bindings/c++/reapi_cli_impl.hpp#L105-L109 

Added lines #L105 - L109 were not covered by tests
}

// Check for an unsuccessful match
if (rc == 0) {
matched = true;
}

if ( (rc = rq->writers->emit (o)) < 0) {
m_err_msg += __FUNCTION__;
Expand All @@ -109,28 +123,6 @@

zekemorton marked this conversation as resolved.
Show resolved Hide resolved
R = o.str ();


reserved = (at != 0)? true : false;
st = (reserved)?
job_lifecycle_t::RESERVED : job_lifecycle_t::ALLOCATED;
if (reserved)
rq->set_reservation (jobid);
else
rq->set_allocation (jobid);

job_info = std::make_shared<job_info_t> (jobid, st, at, "", "", ov);
if (job_info == nullptr) {
errno = ENOMEM;
m_err_msg += __FUNCTION__;
m_err_msg += ": ERROR: can't allocate memory: "
+ std::string (strerror (errno))+ "\n";
rc = -1;
goto out;
}

rq->set_job (jobid, job_info);
rq->incr_job_counter ();

if ( (rc = gettimeofday (&end_time, NULL)) < 0) {
m_err_msg += __FUNCTION__;
m_err_msg += ": ERROR: gettimeofday: "
Expand All @@ -140,6 +132,30 @@

ov = get_elapsed_time (start_time, end_time);

if (matched) {
reserved = (at != 0)? true : false;
st = (reserved)?
job_lifecycle_t::RESERVED : job_lifecycle_t::ALLOCATED;
if (reserved)
rq->set_reservation (jobid);
else
rq->set_allocation (jobid);

job_info = std::make_shared<job_info_t> (jobid, st, at, "", "", ov);
if (job_info == nullptr) {
errno = ENOMEM;
m_err_msg += __FUNCTION__;
m_err_msg += ": ERROR: can't allocate memory: "
+ std::string (strerror (errno))+ "\n";
rc = -1;
goto out;

Check warning on line 151 in resource/reapi/bindings/c++/reapi_cli_impl.hpp

View check run for this annotation

Codecov / codecov/patch

resource/reapi/bindings/c++/reapi_cli_impl.hpp#L146-L151 

Added lines #L146 - L151 were not covered by tests
}
rq->set_job (jobid, job_info);

}

rq->incr_job_counter ();
jameshcorbett marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current behavior in resource-query increments the counter if print_schedule_info is called:

flux-sched/resource/utilities/command.cpp

Line 236 in 0b70113

print_schedule_info (ctx, out, jobid,
It's executed unless:

  1. gettimeofday fails here, or here
  2. The match subcommand isn't recognized
  3. The writer fails to output
  4. The satisfiability check is performed

I think placing rq->incr_job_counter (); before out: will generate the desired behavior or enable it to be generated when new rq2 features are added:

Suggested change
rq->incr_job_counter ();
rq->incr_job_counter ();
out:

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out! I think this change will mean that we will not increment when a match is not found. I can add the increment into the block for that check though


out:
return rc;
}
Expand Down Expand Up @@ -234,6 +250,36 @@
return 0;
}

int reapi_cli_t::info (void *h, const uint64_t jobid,
std::shared_ptr<job_info_t> &job)
{
resource_query_t *rq = static_cast<resource_query_t *> (h);

if ( !(rq->job_exists (jobid))) {
m_err_msg += __FUNCTION__;
m_err_msg += ": ERROR: nonexistent job "
+ std::to_string (jobid) + "\n";
return -1;

Check warning on line 262 in resource/reapi/bindings/c++/reapi_cli_impl.hpp

View check run for this annotation

Codecov / codecov/patch

resource/reapi/bindings/c++/reapi_cli_impl.hpp#L259-L262 

Added lines #L259 - L262 were not covered by tests
}

job = rq->get_job (jobid);
return 0;
}

unsigned int reapi_cli_t::preorder_count (void *h)
{
resource_query_t *rq = static_cast<resource_query_t *> (h);

return rq->preorder_count ();
}

unsigned int reapi_cli_t::postorder_count (void *h)
{
resource_query_t *rq = static_cast<resource_query_t *> (h);

return rq->postorder_count ();
}

int reapi_cli_t::stat (void *h, int64_t &V, int64_t &E,int64_t &J,
double &load, double &min, double &max, double &avg)
{
Expand Down Expand Up @@ -605,6 +651,16 @@
return reservations.find (jobid) != reservations.end ();
}

const unsigned int resource_query_t::preorder_count ()
{
return traverser->get_total_preorder_count ();
}

const unsigned int resource_query_t::postorder_count ()
{
return traverser->get_total_postorder_count ();
}

void resource_query_t::clear_resource_query_err_msg ()
{
m_err_msg = "";
Expand Down
18 changes: 15 additions & 3 deletions resource/reapi/bindings/go/src/test/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
func main() {
jgfPtr := flag.String("jgf", "", "path to jgf")
jobspecPtr := flag.String("jobspec", "", "path to jobspec")
reserve := flag.Bool("reserve", false, "or else reserve?")
reserve := false
flag.Parse()

jgf, err := os.ReadFile(*jgfPtr)
Expand All @@ -44,20 +44,32 @@ func main() {
}
fmt.Printf("Jobspec:\n %s\n", jobspec)

reserved, allocated, at, overhead, jobid, err := cli.MatchAllocate(*reserve, string(jobspec))
reserved, allocated, at, overhead, jobid, err := cli.MatchAllocate(reserve, string(jobspec))
if err != nil {
fmt.Printf("Error in ReapiClient MatchAllocate: %v\n", err)
return
}
printOutput(reserved, allocated, at, jobid, err)
reserved, allocated, at, overhead, jobid, err = cli.MatchAllocate(*reserve, string(jobspec))

reserve = true
reserved, allocated, at, overhead, jobid, err = cli.MatchAllocate(reserve, string(jobspec))
fmt.Println("Errors so far: \n", cli.GetErrMsg())

if err != nil {
fmt.Printf("Error in ReapiClient MatchAllocate: %v\n", err)
return
}
printOutput(reserved, allocated, at, jobid, err)

reserved, allocated, at, overhead, jobid, err = cli.MatchAllocate(reserve, string(jobspec))
fmt.Println("Errors so far: \n", cli.GetErrMsg())

if err != nil {
fmt.Printf("Error in ReapiClient MatchAllocate: %v\n", err)
return
}
printOutput(reserved, allocated, at, jobid, err)

err = cli.Cancel(1, false)
if err != nil {
fmt.Printf("Error in ReapiClient Cancel: %v\n", err)
Expand Down
8 changes: 8 additions & 0 deletions resource/utilities/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ target_link_libraries(resource-query
Boost::filesystem
Boost::headers
)
add_executable(rq2
rq2.cpp
rq2.hpp
)
target_link_libraries(rq2
resource
PkgConfig::LIBEDIT
)

add_subdirectory(test)

Loading
Loading