tmt icon indicating copy to clipboard operation
tmt copied to clipboard

Change the default unit of memory HW requirement

Open skycastlelily opened this issue 1 year ago • 8 comments

If users specify memory: > 4096 ,when they want to get an instance with memory > 4096 MiB, tmt will provide memory > 0 instance.

skycastlelily avatar Jul 26 '24 08:07 skycastlelily

If users specify memory: > 4096 ,when they want to get an instance with memory > 4096 MiB

Maybe, who knows what user wanted :) The should have been more specific :)

tmt will provide memory > 0 instance.

Referenced PR changes the default unit - might be good, might be bad, who knows, but I don't understand how it addresses the issue with > 0. IIUIC, > 4096 should provision an instance with more than 4096 bytes of RAM, which should translate into an instance with let's say 2 GB of RAM. Something very small, but definitely not zero. Can you share more details about this? It seems like two distinct problems in this issue - default unit change, plus maybe a bug in how > 4096 is evaluated today.

happz avatar Jul 26 '24 08:07 happz

If users specify memory: > 4096 ,when they want to get an instance with memory > 4096 MiB

Maybe, who knows what user wanted :) The should have been more specific :)

tmt will provide memory > 0 instance.

Referenced PR changes the default unit - might be good, might be bad, who knows, but I don't understand how it addresses the issue with > 0. IIUIC, > 4096 should provision an instance with more than 4096 bytes of RAM, which should translate into an instance with let's say 2 GB of RAM. Something very small, but definitely not zero. Can you share more details about this? It seems like two distinct problems in this issue - default unit change, plus maybe a bug in how > 4096 is evaluated today.

Edit: when a user says disk.size: "> 40", do they mean bytes or do they mean GB? As you can see, the change of default unit in the name of what the user might mean should probably cover disk.size as well, but then again, it seems to be not connected to how > 4096 turns into a "memory > 0 instance", which is unclear to me & feels really weird, might be a bug.

happz avatar Jul 26 '24 08:07 happz

Added as a topic for next week's hacking session agenda - fixing evaluation of > 4096 vs > 0 seems like a bug to fix, the change of default unit needs more discussion as it's a user-facing specification.

happz avatar Jul 26 '24 09:07 happz

There is no bug for how 4096 is translated, mrack use to('MiB').magnitude just as artemis does: 4096/1024/1024 = 0.0039 ~ 0 ^^

On Fri, Jul 26, 2024 at 5:04 PM Miloš Prchlík @.***> wrote:

Added as a topic for next week's hacking session agenda - fixing evaluation of > 4096 vs > 0 seems like a bug to fix, the change of default unit needs more discussion as it's a user-facing specification.

— Reply to this email directly, view it on GitHub https://github.com/teemtee/tmt/issues/3112#issuecomment-2252295439, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKFR23EFOH6VGRGOBAVZCNDZOIGJXAVCNFSM6AAAAABLQCQME6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJSGI4TKNBTHE . You are receiving this because you authored the thread.Message ID: @.***>

skycastlelily avatar Jul 26 '24 09:07 skycastlelily

I see. So it is just a topic for discussion whether the default unit of memory HW requirement should change and what it should be.

happz avatar Jul 26 '24 09:07 happz

Yeah,please note though beaker accepts value+unit through web page(uses MiB if no unit is specified), but beaker job doesn't accept memory unit, and it will treat the value as MiB sized:)

On Fri, Jul 26, 2024 at 5:46 PM Miloš Prchlík @.***> wrote:

I see. So it is just a topic for discussion whether the default unit of memory HW requirement should change and what it should be.

— Reply to this email directly, view it on GitHub https://github.com/teemtee/tmt/issues/3112#issuecomment-2252371424, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKFR23HZT3GOTZOFLZ5LRPDZOILGXAVCNFSM6AAAAABLQCQME6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJSGM3TCNBSGQ . You are receiving this because you authored the thread.Message ID: @.***>

skycastlelily avatar Jul 26 '24 10:07 skycastlelily

@skycastlelily would you mind changing the subject of the issue then, to something like "Change the default unit of memory HW requirement"? IIUIC, the translation is perfectly fine, and memory: 4096 is correctly converted into zero, because, as the specification says, the default is bytes. Maybe we could add a check for unexpected sizes, like Beaker translating it to 0 MB would emit a warning, for example.

happz avatar Jul 26 '24 10:07 happz

Sure:)

skycastlelily avatar Jul 29 '24 00:07 skycastlelily