Make LDMSD support interval strings #1242
Conversation
|
@nick-enoent Here is the patch that removes |
ldms/man/ldmsd_controller.man
Outdated
| us -- microseconds | ||
| ms -- milliseconds | ||
| s -- seconds | ||
| min -- minutes |
There was a problem hiding this comment.
'm'
'h'
'd'
seems more concise.
There was a problem hiding this comment.
There was a problem hiding this comment.
@tom95858 @nick-enoent I changed the minute, hour, and day strings to 'm', 'h', and 'd', respectively. Also, the patch supports floats, e.g., '1.5s' as in Maestro.
|
@nichamon, @nick-enoent can we make sure whatever these strings are they need to be match what maestro uses. |
@tom95858 After I shorten 'min' to 'm', the strings in this patch will be the same as in Maestro. However, there are 2 main differences:
I will update the patch during the weekend. |
|
@nichamon The only one I think is unreasonable is nano-second. |
There was a problem hiding this comment.
@nichamon This patch works just fine with maestro currently, aside from the "m" vs "min" format for minutes.
nichamon
force-pushed
the
support-interval-string
branch
3 times, most recently
from
a38b0c8
to
ea35794
Compare
The patch makes LDMSD support time-interval strings. A time-interval string is a float followed by a unit string. A unit string is one of the followings: us -- microseconds ms -- milliseconds s -- seconds m -- minutes h -- hours d -- days For example, with this patch, '1s', '1.5s', and '100'ms' can be used.
nichamon
force-pushed
the
support-interval-string
branch
from
ea35794
to
6a8261a
Compare
|
@tom95858 Per your request in #1240 (comment), I'll add this patch on top of #1240 and close this pull request when we think this patch is good . |
|
@nichamon, @nick-enoent is this patch ready to go? |
| @@ -204,17 +204,6 @@ | |||
| 'auth_add': {'req_attr': ['name', 'plugin'], 'opt_attr': []}, | |||
| } | |||
|
|
|||
| def check_offset(interval_us, offset_us=None): | |||
There was a problem hiding this comment.
Why did we decide that this check was no longer necessary?
There was a problem hiding this comment.
- The main reason is that LDMSD also checks the offset value.
- With the patch, LDMSD converts interval strings to microseconds, so we don't have to duplicate the code in ldmsctl, ldmsd_controller, and the config file parser.
- If we want to keep the check_offset(), I need to convert interval strings to microseconds before passing the values to check_offset(). I will probably move the conversion and the check logic out from LDMSD and put them in ldmsctl and config file parser, similar to ldmsd_controller.
|
@nichamon, ready to merge? |
Yes, it is ready. |
The patch makes LDMSD support time-interval strings. A time-interval string is an integer followed by a unit string. A unit string is one of the followings:
us -- microseconds
ms -- milliseconds
s -- seconds
min -- minutes
hr -- hours
day -- days
For example, with this patch, to specify a sampling interval of 1 second, '1s' can be used.