Skip to content
This repository has been archived by the owner on Oct 22, 2024. It is now read-only.

Dev-manager canUse calc: fix operator order #717

Merged
merged 2 commits into from
Aug 31, 2020

Conversation

okartau
Copy link
Contributor

@okartau okartau commented Aug 28, 2020

Division as integer and then multiply, sheds off some value
from canUse size, and later when we align down by Align size,
we will lose whole alignment block because of that.
This creates issue where 1st run leaves one realAlign-sized block
unallocated, but 2nd start on same node will find it and create
a new relatively small namespace.

Division as integer and then multiply, sheds off some value
from canUse size, and later when we align down by Align size,
we will lose whole alignment block because of that.
This creates issue where 1st run leaves one realAlign-sized block
unallocated, but 2nd start on same node will find it and create
a new relatively small namespace.
Copy link
Contributor

@pohly pohly left a comment

Choose a reason for hiding this comment

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

Not strictly related, but can you use the opportunity to explain what "realalign" is? It's not obvious when reading this function.

The fix itself looks fine. Unit tests would be good, but we are already tracking that (issue #692 ).

@pohly
Copy link
Contributor

pohly commented Aug 28, 2020

Not strictly related, but can you use the opportunity to explain what "realalign" is?

And with that I meant "explain in the source code" - not here in the GitHub issue.

@okartau
Copy link
Contributor Author

okartau commented Aug 28, 2020

testing failed but I believe it is not related to this change:

The Service "pmem-csi-scheduler" is invalid: spec.ports[0].nodePort: Invalid value: 32000: provided port is already allocated
[AfterEach] direct-testing-metrics

I will add another commit with asked "realalign" explanation.

@pohly pohly merged commit b8b7db6 into intel:devel Aug 31, 2020
@pohly
Copy link
Contributor

pohly commented Sep 1, 2020

The Service "pmem-csi-scheduler" is invalid: spec.ports[0].nodePort: Invalid value: 32000: provided port is already allocated
[AfterEach] direct-testing-metrics

I just hit the same locally. Seems to be timing issue with plenty of upstream issues that report it. I'll probably add a retry loop.

@okartau okartau deleted the fix-size-calc-order branch March 16, 2021 14:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants