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

Add comments relating memory sizing. #88

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

matthewvon
Copy link
Member

@matthewvon matthewvon commented Nov 14, 2022

Adding comments about memory sizing in README.md and values.yaml.

These changes were part of the discussions to correct a customer issue in PLAT-5010. (Not naming customer since this is a public repo.)

@matthewvon matthewvon requested a review from pdmars November 14, 2022 15:11
Copy link
Member

@pdmars pdmars left a comment

Choose a reason for hiding this comment

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

Looks good. Two small nitpicks.

Memory settings are part of both javaArgs and resources within
values.yaml. The settings must be coordinated.

Stardog will aggressive use all memory allocated to it via the
Copy link
Member

Choose a reason for hiding this comment

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

s/aggressive/aggressively

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Thank you.

must be at least 1g larger than -Xmx and -XX:MaxDirectMemorySize total. A
too small limits:memory setting will likely result in a system crash.

Network based disk storage is not recommended. If used, the
Copy link
Member

Choose a reason for hiding this comment

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

Can you clarify with some examples here vs block storage

Copy link
Member Author

Choose a reason for hiding this comment

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

Rewrote the network storage section completely differently. And remembered we used a different approach with Disney and Windows SMB.

@matthewvon matthewvon requested a review from pdmars November 14, 2022 17:40
@matthewvon matthewvon requested a review from ctffarley November 14, 2022 17:43

Network based disk storage is not recommended. This include NFS, SMB, and
EFS based storage systems.

Copy link
Contributor

Choose a reason for hiding this comment

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

On AWS, we recommend gp2 as the storageClass. On GKS and AKS standard (the default) is what we recommend. I would just add something like "we instead recommend setting the storageClass to gp2 on AWS and leaving the default values of standard for Azure and GCP."

Copy link
Member

Choose a reason for hiding this comment

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

Just to clarify from discussions outside of this PR. I think we should call out a bit more details that the storage classes may not always be named this depending on how they are configured. Instead we should tell people to use block volume storage classes for their infrastructure and depending on their use case they may want to explore differences between the types, e.g. gp2 vs io2 or standard vs premium. We can also link to the k8s docs on storage classes for further information: https://kubernetes.io/docs/concepts/storage/storage-classes/

Copy link
Contributor

Choose a reason for hiding this comment

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

To include the suggestions from Paul's comment, I would edit it to read something like:
Stardog instead recommends using block volume storage classes such as gp2 on AWS or standard on other platforms. Depending on use case, it is recommended to explore differences between the types, e.g. gp2 vs io2 or standard vs premium.

@matthewvon matthewvon requested a review from ctffarley November 14, 2022 18:30

Network based disk storage is not recommended. This include NFS, SMB, and
EFS based storage systems.

Copy link
Contributor

Choose a reason for hiding this comment

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

To include the suggestions from Paul's comment, I would edit it to read something like:
Stardog instead recommends using block volume storage classes such as gp2 on AWS or standard on other platforms. Depending on use case, it is recommended to explore differences between the types, e.g. gp2 vs io2 or standard vs premium.

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.

3 participants