[
https://issues.apache.org/jira/browse/YUNIKORN-1403?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17733995#comment-17733995
]
Wilfred Spiegelenburg commented on YUNIKORN-1403:
-------------------------------------------------
There is a slight behaviour difference between the hardware platforms. The test
case above shows that, ARM64 and AMD64 give different values back. Even if you
run it with the 100 inside the conversion it still shows differences. It was
not intentional to have the 100 outside of the conversion, it should have been
within. That also fixes the 0 value as the conversion happens after we have
done the multiplication. The multiplication seems to "hide" some of the issue.
We need to work and detect this correctly on all platforms not just AMD64. This
might require a test that would only ever fail on ARM64
I was not happy with the tests as they were not picking up all issues. Based on
that I have rewritten the test code: [https://go.dev/play/p/8Q6yStV_sPB]
I have moved all calculations into the float line and then convert in one step.
This allows picking up the multiply overflow, both negative and positive sides,
in a simple check of the float against the max and min value of an int64. As
you can see in the code comments around the infinite and multiply overflow
cases the behaviour is not consistent between the platforms.
I think we need to move to the following detection mechanism:
{code:java}
div := float64(usedResource) * 100 / float64(availableResource)
absResValue := int64(div)
if ((usedResource > 0) == (availableResource > 0)) == (absResValue < 0) || (div
> math.MaxInt64) || (div < math.MinInt64) {
log.Logger().Warn("Absolute resource value result wrapped or overflow",
zap.String("resource", resourceName),
zap.Int64("capacity", int64(availableResource)),
zap.Int64("usage", int64(usedResource)))
}{code}
This does remove the setting of absResValue as that was unneeded. The int
conversion has made a choice and we can live with that choice as the value is
not really correct.
> CalculateAbsUsedCapacity: overflow unit tests
> ---------------------------------------------
>
> Key: YUNIKORN-1403
> URL: https://issues.apache.org/jira/browse/YUNIKORN-1403
> Project: Apache YuniKorn
> Issue Type: Improvement
> Components: core - common, test - unit
> Reporter: Wilfred Spiegelenburg
> Priority: Major
> Labels: newbie, pull-request-available
>
> CalculateAbsUsedCapacity is guarded against positive and negative overflow.
> The postive case is unit tested. The negative case is not unit tested.
> Need to add a test case for te negative side:
> {code}
> // protect against negative integer overflow
> if absResValue > 0 && div < 0 {
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
