Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use smaller disks by default for tests on Azure #272

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

zhaocongjia
Copy link
Collaborator

Copy link
Contributor

@lilyeyes lilyeyes left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -212,7 +212,7 @@ variable "hana_data_disks_configuration" {
type = map(any)
default = {
disks_type = "Premium_LRS,Premium_LRS,Premium_LRS,Premium_LRS,Premium_LRS,Premium_LRS,Premium_LRS"
disks_size = "128,128,128,128,128,128,128"
disks_size = "32,32,16,16,32,64,32"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can add a comment in the code to say something like

This default is aligned to default for hana_vm_size

We can also update comment in line 197 to talk about Standard_E4s_v3 and not Standard_M32ls

Copy link
Collaborator

@mpagot mpagot Sep 11, 2024

Choose a reason for hiding this comment

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

Each value now is using a different number: does it make sense to document why and which disk has which size? Where we should do it? Here in this file or in the readme?

There's an explanation here

its data storage `/hana/data`, `/hana/log`, `/hana/shared`, `/usr/sap` and `/hana/backup`.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think those disks sizes are not finalized. I'll collect some data about cost saving with this commit. Then there shall be a follow up ticket to optimize the disk sizes further. Then we can add definitive documentation about disk sizes.

@mpagot
Copy link
Collaborator

mpagot commented Sep 11, 2024

We should update and expand documentation in

| **Demo** | 2x128GB LUN 0,1 | 2x128GB LUN 2,3 | 128GB LUN 4 | 128GB LUN 5 | 128GB LUN 6 |
and
disks_size = "128,128,128,128,128,128,128"

@zhaocongjia
Copy link
Collaborator Author

Good idea, I should keep the code and documentation in sync.

@zhaocongjia
Copy link
Collaborator Author

Copy link
Collaborator

@alvarocarvajald alvarocarvajald left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@mpagot mpagot left a comment

Choose a reason for hiding this comment

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

LGTM

@mpagot mpagot merged commit eebfa05 into SUSE:main Sep 12, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants