Ticket 2894

I'm passing this over to Alex who wrote the plugin; he should get back to you when he's back from vacation in a week or so.

Thanks Tim.

Hi Davide. You want the result of start_time - eligible_time to be indexed. Problem is all the job info provided to the plugin upon job completion is job_record struct, which doesn't contain eligible_time. Neither job_details struct which is inside job_record.

My initial approach (before fff593958) was start_time - begin_time, since begin_time is supposedly resetted to eligible_time at some point:

time_t begin_time;              /* start at this time (srun --begin),
                                 * resets to time first eligible
                                 * (all dependencies satisfied) */

but start_time - begin_time lead to a user sending me an email complaining about the result not being able to be indexed. From the elasticsearch logs:

[2016-06-10 09:43:10,467][DEBUG][action.index             ] [solees01-es1] [slurm][1], node[TE3vix5IR_qdd-y5_hYgEA], [P], s[STARTED]: Failed to execute [index {[slurm][jobcomp]
...
"eligible_time":18446744073709551615
...
org.elasticsearch.index.mapper.MapperParsingException: failed to parse [eligible_time]
...
Caused by: org.elasticsearch.common.jackson.core.JsonParseException: Numeric value (18446744073709551615) out of range of long (-9223372036854775808 - 9223372036854775807)

So I decided to remove that field. It seems that somehow the original code:

-       if (job_ptr->details && (job_ptr->details->begin_time != NO_VAL)) {
-		eligible_time = job_ptr->start_time -
-				job_ptr->details->begin_time;
-		xstrfmtcat(buffer, ",\"eligible_time\":%lu", eligible_time);
-	}

resulted in some cases slurm trying to index 18446744073709551615 in elasticsearch, which is the highest 64 bit unsigned value (2^64)-1. Reading the elastic documentation[1], maximum value is signed long:

A signed 64-bit integer with a minimum value of -(2^63) and a maximum value of (2^63)-1, which corresponds to the out of range of long error in the elasticsearch log. so these two reasons are why I decided to remove this field, and also because it might cause confusion since eligible_time was already a name designed with another meaning inside Slurm, and not something defined as start_time - begin_time.

Now with all these, would it work for you if I create a patch so that the result from start_time - submit_time is indexed? At the end of the day what you might want is the time a job is waiting before it starts since it was submitted, isn't it? I could do some tests with this and call this field start_delay, and before indexing make sure its value is inside the bounds of elasticsearch. Please, let me know if this would be a valid solution. Thanks.

[1] https://www.elastic.co/guide/en/elasticsearch/reference/current/number.html

Hi Alejandro,
yes, what we need to monitor is the time elapsed from the job submission to the job start.
As I said before, I totally understand the reason behind your choice of removing that field (I followed it via github) but I don't think that removing it altogether is the right solution. That metrics is very important to monitor the status of the cluster and not having it would be very limiting for us. And I don't think we are the only ones interested on monitoring the queue wait time. 
So if I understand correctly the alternative solutions are the following:

1) Extend the job_record struct so that it includes eligible_time

2) Check the value of eligible_time before sending it to ES.

I would prefer the first since it would eliminate the eventuality of spurious data altogether. But I can be happy with the second too.
I also agree with you that eligible_time is a pretty non-intuitive name. It took me some time to realize it was what I wanted. I would suggest pending_time, queue_time or their permutations.

Another couple of suggestions on future enhancements:

Have you ever considered splitting the indexes by time interval (which can be user-defined in slurm.conf) in a logstash-like fashion? For example in our case it would be nice to have the possibility of having one index per month. That would speed up queries significantly because I don't have to query the whole database if I'm interested only in a specific set of months. Plus the shards would be better distributed across the ES cluster, improving performance and data integrity.

Another problem is the fact that for multiple nodes allocation the nodes field looks like this:

vmp[304,309,348,378,398,431,479,481]

Since the nodes field is analyzed, when searching for nodes it cannot (obviously) expand the nodes list, making the generation of statistics for a specific set of nodes impossible. What do you think about replacing the contracted form with a space separated list of hostnames?

Davide

Hi Davide. Following commit adds a new 'start_delay' field defined as the start_time - submit_time delay period. Note we committed it against master branch (future 17.02). Hopefully the sanity checks before indexing the field and the usage of the difftime function should make indexing this field something safe. I did quiet a few use case tests, and at least locally it works as expected. Let me know if you encounter any issue.

https://github.com/SchedMD/slurm/commit/e84398809fab1

Regarding the other two suggestions:

1) Have a more fine-grained and configurable through slurm.conf index splitting. Something like ElasticSearchIndex=[none|day|week|month|year] or something to be discussed.

2) Change 'nodes' field to expanded form, or maybe even better, let user configure if expanded/contracted is needed, since other users might consider the expanded form too verbose when jobs are allocated huge amounts of nodes.

I think these two suggestions are good ideas. I'm switching severity to 5 enhancement, but notice that it's not something we're committing to since enhancements don't fall under our normal support contract, but if you're interested in sponsoring any of these features please let as know and we can discuss that further.

So all in all, please let me know if you encounter any issues with the 'start_delay' and then your position regarding the enhancements.

Thanks!

Alejandro,
thanks for bringing it back. Is there a way to port it to the 16.05?

I forked your master branch so that I can start taking a look at how the elasticsearch plugin is implemented and, if time permits, to provide some suggestions on how to apply those enhancements.

Davide

(In reply to Davide Vanzo from comment #11)
> Alejandro,
> thanks for bringing it back. Is there a way to port it to the 16.05?

Davide, following commit[1] adds eligible_time also as a time stamp field and refactors the way 'start_delay' (now called 'queue_wait') is calculated, based upon start_time and begin_time, which I agree with Danny is the correct way to do it. Patch will be added to Slurm 16.05.3 (which should be released today) and also is merged into master branch.

[1] https://github.com/SchedMD/slurm/commit/0a4d5770e

> I forked your master branch so that I can start taking a look at how the
> elasticsearch plugin is implemented and, if time permits, to provide some
> suggestions on how to apply those enhancements.
> 
> Davide

Ok. For the moment remember these other 2 enhancements are *not* something we're committing to sine they don't fall under our normal support contract. Just let me know if you're interested in sponsorship and we could discuss the details further. Thanks.

Could you give me some more information about sponsorship?
Thanks

Davide

Davide, we think these features are possible to implement. Jacob will be contacting you about potential sponsorship.