Rapl node capping daemon #21
|
|
@ -61,8 +61,7 @@ func capNode(base string, percentage int) error {
|
|||
Can you expand on this comment? Can you expand on this comment?
Actually, your way of representing the ranges is more explicit and clear. One is not well-versed with range expressions would still be able to understand your explanation. So, ignore my previous statement and leave it as is. Actually, your way of representing the ranges is more explicit and clear. One is not well-versed with range expressions would still be able to understand your explanation. So, ignore my previous statement and leave it as is.
Either change overflows to overflow, or get rid of the "an". Either change **overflows** to **overflow**, or get rid of the "an".
Good catch, fixed. Good catch, fixed.
Fixed this, definitely a mistake on my part. Thanks for catching it! Fixed this, definitely a mistake on my part. Thanks for catching it!
I'll revert it I'll revert it
constraint_1 is the shorter window. constraint_1 is the shorter window.
maybe just say cap percentage must be in the range (0, 100]. maybe just say **cap percentage must be in the range (0, 100]**.
constraint_0 is the longer window. constraint_0 is the longer window.
Going by [this](https://github.com/spdfg/elektron/blob/master/scripts/RAPL_PKG_Throttle.py#L48) we are intending to use the longer window for running average calculations.
So, change the variable name to `maxPowerFileLongWindow`?
ditto. ditto.
overflow overflow~s~
fits on a single line. fits on a single line.
Can you expand on this comment? Can you expand on this comment?
Actually, your way of representing the ranges is more explicit and clear. One is not well-versed with range expressions would still be able to understand your explanation. So, ignore my previous statement and leave it as is. Actually, your way of representing the ranges is more explicit and clear. One is not well-versed with range expressions would still be able to understand your explanation. So, ignore my previous statement and leave it as is.
Either change overflows to overflow, or get rid of the "an". Either change **overflows** to **overflow**, or get rid of the "an".
Good catch, fixed. Good catch, fixed.
Fixed this, definitely a mistake on my part. Thanks for catching it! Fixed this, definitely a mistake on my part. Thanks for catching it!
I'll revert it I'll revert it
|
||||
return nil
|
||||
}
|
||||
|
||||
// maxPower returns the value in float of the maximum watts a power zone
|
||||
|
constraint_1 is the shorter window. constraint_1 is the shorter window.
maybe just say cap percentage must be in the range (0, 100]. maybe just say **cap percentage must be in the range (0, 100]**.
constraint_0 is the longer window. constraint_0 is the longer window.
Going by [this](https://github.com/spdfg/elektron/blob/master/scripts/RAPL_PKG_Throttle.py#L48) we are intending to use the longer window for running average calculations.
So, change the variable name to `maxPowerFileLongWindow`?
ditto. ditto.
overflow overflow~s~
fits on a single line. fits on a single line.
Can you expand on this comment? Can you expand on this comment?
Actually, your way of representing the ranges is more explicit and clear. One is not well-versed with range expressions would still be able to understand your explanation. So, ignore my previous statement and leave it as is. Actually, your way of representing the ranges is more explicit and clear. One is not well-versed with range expressions would still be able to understand your explanation. So, ignore my previous statement and leave it as is.
Either change overflows to overflow, or get rid of the "an". Either change **overflows** to **overflow**, or get rid of the "an".
Good catch, fixed. Good catch, fixed.
Fixed this, definitely a mistake on my part. Thanks for catching it! Fixed this, definitely a mistake on my part. Thanks for catching it!
I'll revert it I'll revert it
|
||||
// can use.
|
||||
|
constraint_1 is the shorter window. constraint_1 is the shorter window.
maybe just say cap percentage must be in the range (0, 100]. maybe just say **cap percentage must be in the range (0, 100]**.
constraint_0 is the longer window. constraint_0 is the longer window.
Going by [this](https://github.com/spdfg/elektron/blob/master/scripts/RAPL_PKG_Throttle.py#L48) we are intending to use the longer window for running average calculations.
So, change the variable name to `maxPowerFileLongWindow`?
ditto. ditto.
overflow overflow~s~
fits on a single line. fits on a single line.
Can you expand on this comment? Can you expand on this comment?
Actually, your way of representing the ranges is more explicit and clear. One is not well-versed with range expressions would still be able to understand your explanation. So, ignore my previous statement and leave it as is. Actually, your way of representing the ranges is more explicit and clear. One is not well-versed with range expressions would still be able to understand your explanation. So, ignore my previous statement and leave it as is.
Either change overflows to overflow, or get rid of the "an". Either change **overflows** to **overflow**, or get rid of the "an".
Good catch, fixed. Good catch, fixed.
Fixed this, definitely a mistake on my part. Thanks for catching it! Fixed this, definitely a mistake on my part. Thanks for catching it!
I'll revert it I'll revert it
|
||||
// maxPower returns the value in float of the maximum watts a power zone can use.
|
||||
|
constraint_1 is the shorter window. constraint_1 is the shorter window.
maybe just say cap percentage must be in the range (0, 100]. maybe just say **cap percentage must be in the range (0, 100]**.
constraint_0 is the longer window. constraint_0 is the longer window.
Going by [this](https://github.com/spdfg/elektron/blob/master/scripts/RAPL_PKG_Throttle.py#L48) we are intending to use the longer window for running average calculations.
So, change the variable name to `maxPowerFileLongWindow`?
ditto. ditto.
overflow overflow~s~
fits on a single line. fits on a single line.
Can you expand on this comment? Can you expand on this comment?
Actually, your way of representing the ranges is more explicit and clear. One is not well-versed with range expressions would still be able to understand your explanation. So, ignore my previous statement and leave it as is. Actually, your way of representing the ranges is more explicit and clear. One is not well-versed with range expressions would still be able to understand your explanation. So, ignore my previous statement and leave it as is.
Either change overflows to overflow, or get rid of the "an". Either change **overflows** to **overflow**, or get rid of the "an".
Good catch, fixed. Good catch, fixed.
Fixed this, definitely a mistake on my part. Thanks for catching it! Fixed this, definitely a mistake on my part. Thanks for catching it!
I'll revert it I'll revert it
|
||||
func maxPower(maxFile string) (uint64, error) {
|
||||
maxPower, err := ioutil.ReadFile(maxFile)
|
||||
if err != nil {
|
||||
|
|
|
|||
|
constraint_1 is the shorter window. constraint_1 is the shorter window.
maybe just say cap percentage must be in the range (0, 100]. maybe just say **cap percentage must be in the range (0, 100]**.
constraint_0 is the longer window. constraint_0 is the longer window.
Going by [this](https://github.com/spdfg/elektron/blob/master/scripts/RAPL_PKG_Throttle.py#L48) we are intending to use the longer window for running average calculations.
So, change the variable name to `maxPowerFileLongWindow`?
ditto. ditto.
overflow overflow~s~
fits on a single line. fits on a single line.
Can you expand on this comment? Can you expand on this comment?
Actually, your way of representing the ranges is more explicit and clear. One is not well-versed with range expressions would still be able to understand your explanation. So, ignore my previous statement and leave it as is. Actually, your way of representing the ranges is more explicit and clear. One is not well-versed with range expressions would still be able to understand your explanation. So, ignore my previous statement and leave it as is.
Either change overflows to overflow, or get rid of the "an". Either change **overflows** to **overflow**, or get rid of the "an".
Good catch, fixed. Good catch, fixed.
Fixed this, definitely a mistake on my part. Thanks for catching it! Fixed this, definitely a mistake on my part. Thanks for catching it!
I'll revert it I'll revert it
constraint_1 is the shorter window. constraint_1 is the shorter window.
maybe just say cap percentage must be in the range (0, 100]. maybe just say **cap percentage must be in the range (0, 100]**.
constraint_0 is the longer window. constraint_0 is the longer window.
Going by [this](https://github.com/spdfg/elektron/blob/master/scripts/RAPL_PKG_Throttle.py#L48) we are intending to use the longer window for running average calculations.
So, change the variable name to `maxPowerFileLongWindow`?
ditto. ditto.
overflow overflow~s~
fits on a single line. fits on a single line.
Can you expand on this comment? Can you expand on this comment?
Actually, your way of representing the ranges is more explicit and clear. One is not well-versed with range expressions would still be able to understand your explanation. So, ignore my previous statement and leave it as is. Actually, your way of representing the ranges is more explicit and clear. One is not well-versed with range expressions would still be able to understand your explanation. So, ignore my previous statement and leave it as is.
Either change overflows to overflow, or get rid of the "an". Either change **overflows** to **overflow**, or get rid of the "an".
Good catch, fixed. Good catch, fixed.
Fixed this, definitely a mistake on my part. Thanks for catching it! Fixed this, definitely a mistake on my part. Thanks for catching it!
I'll revert it I'll revert it
|
||||
|
|
@ -35,12 +35,12 @@ func TestMain(m *testing.M) {
|
|||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
|
||||
initialWatts := strconv.FormatUint(maxWattage, 10)
|
||||
|
||||
err = ioutil.WriteFile(filepath.Join(zonePath, maxPowerFileShortWindow), []byte(initialWatts), 0444)
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
err = ioutil.WriteFile(filepath.Join(zonePath, maxPowerFileLongWindow), []byte(initialWatts), 0444)
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
if err != nil {
|
||||
log.Fatal(err)
|
||||
}
|
||||
|
||||
err = ioutil.WriteFile(filepath.Join(zonePath, powerLimitFileShortWindow), []byte(initialWatts), 0644)
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
err = ioutil.WriteFile(filepath.Join(zonePath, powerLimitFileLongWindow), []byte(initialWatts), 0644)
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
if err != nil {
|
||||
log.Fatal(err)
|
||||
}
|
||||
|
|
@ -48,30 +48,29 @@ func TestMain(m *testing.M) {
|
|||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
os.Exit(m.Run())
|
||||
}
|
||||
|
||||
// TODO(rdelvalle): Create filesystem only once and allow tests to use it
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
func TestCapNode(t *testing.T) {
|
||||
err := capNode(raplDir, 95)
|
||||
assert.NoError(t, err)
|
||||
|
||||
t.Run("badPercentage", func(t *testing.T) {
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
t.Run("bad-percentage", func(t *testing.T) {
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
err := capNode(raplDir, 1000)
|
||||
assert.Error(t, err)
|
||||
})
|
||||
|
||||
t.Run("zeroPercent", func(t *testing.T) {
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
t.Run("zero-percent", func(t *testing.T) {
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
err := capNode(raplDir, 0)
|
||||
assert.Error(t, err)
|
||||
})
|
||||
}
|
||||
|
||||
func TestMaxPower(t *testing.T) {
|
||||
maxFile := filepath.Join(raplDir, raplPrefixCPU+":0", maxPowerFileShortWindow)
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
maxFile := filepath.Join(raplDir, raplPrefixCPU+":0", maxPowerFileLongWindow)
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
|
||||
maxWatts, err := maxPower(maxFile)
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, maxWattage, maxWatts)
|
||||
|
||||
t.Run("nameDoesNotExist", func(t *testing.T) {
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
t.Run("name-does-not-exist", func(t *testing.T) {
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
_, err := maxPower("madeupname")
|
||||
assert.Error(t, err)
|
||||
})
|
||||
|
|
@ -81,7 +80,7 @@ func TestCapZone(t *testing.T) {
|
|||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
const percentage float64 = .50
|
||||
|
||||
powercap := uint64(math.Ceil(float64(maxWattage) * percentage))
|
||||
limitFile := filepath.Join(raplDir, raplPrefixCPU+":0", powerLimitFileShortWindow)
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
limitFile := filepath.Join(raplDir, raplPrefixCPU+":0", powerLimitFileLongWindow)
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
err := capZone(limitFile, powercap)
|
||||
assert.NoError(t, err)
|
||||
|
||||
|
The payload declares The payload declares `percentage` as an integer. So, we would never hit this scenario.
Should the percentage in the payload be an integer?
The payload does indeed take an integer, but when we calculate powercap, I'd rather multiply by a fraction than to multiply by a (0,100] integer and then divide by 100. This is done in the next line and it's just for testing. The payload does indeed take an integer, but when we calculate powercap, I'd rather multiply by a fraction than to multiply by a (0,100] integer and then divide by 100. This is done in the next line and it's just for testing.
|
||||
|
|
@ -92,7 +91,7 @@ func TestCapZone(t *testing.T) {
|
|||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, powercap, newCap)
|
||||
|
||||
t.Run("nameDoesNotExist", func(t *testing.T) {
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
t.Run("name-does-not-exist", func(t *testing.T) {
|
||||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
err := capZone("madeupname", powercap)
|
||||
assert.Error(t, err)
|
||||
})
|
||||
|
|
|
|||
|
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default. I think it's safe to use spaces ("bad percentage") as the words are hyphenated by default.
Although, I do not mind the camelcasing.
Good point, fixed. Good point, fixed.
|
||||
constraint_1 is the shorter window.
maybe just say cap percentage must be in the range (0, 100].
constraint_0 is the longer window.
Going by this we are intending to use the longer window for running average calculations.
So, change the variable name to
maxPowerFileLongWindow?ditto.
overflow
sfits on a single line.