Skip to content

Conversation

@dwsmith1983
Copy link
Contributor

What changes were proposed in this pull request?

I changed some grammatical issues in the documentation. One potential change could be debatable though so please take a look.
https://spark.apache.org/docs/latest/tuning.html

Why are the changes needed?

Some grammatical mistakes in the documentation.

Does this PR introduce any user-facing change?

Yes, this corrects some issues in documentation related to Tuning Spark. The following changes were made
Check if there are too many garbage collections by collecting GC stats. If a full GC is invoked multiple times for
before a task completes, it means that there isn't enough memory available for executing tasks.
with -XX:G1HeapRegionSize. (added missing period).
we can estimate the size of Eden to be 4*3*128MiB. (added the to estimate the size--this one I guess debatable)

How was this patch tested?

No tests added as this was markdown documentation for the user facing page.

@github-actions github-actions bot added the DOCS label Nov 3, 2022
…points of data locality didnt have commas or periods but periods were being used for multiple sentences in a bullet points. I opted for periods but could be commas with last one using and then period
@dwsmith1983
Copy link
Contributor Author

Do I need to report test results for a doc change? Also, I am not sure I understand how to do that. Thanks for any feedback.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

Copy link
Contributor

@itholic itholic left a comment

Choose a reason for hiding this comment

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

It would be great if the screen-capture for generated documentation is attached to the PR description for documentation work, but I think it's not necessary for this case since the change is small.

@dwsmith1983
Copy link
Contributor Author

It would be great if the screen-capture for generated documentation is attached to the PR description for documentation work, but I think it's not necessary for this case since the change is small.

I can add it. I was reading over this doc which is why I noticed them. Just a render markdown screenshot or the text file? Or side by side? @itholic

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

This is fine, but fairly trivial - let's do one big PR with many docs if going to do this

@srowen srowen closed this in 5d7be08 Nov 6, 2022
@srowen
Copy link
Member

srowen commented Nov 6, 2022

Merged to master

SandishKumarHN pushed a commit to SandishKumarHN/spark that referenced this pull request Dec 12, 2022
### What changes were proposed in this pull request?

I changed some grammatical issues in the documentation. One potential change could be debatable though so please take a look.
https://spark.apache.org/docs/latest/tuning.html

### Why are the changes needed?

Some grammatical mistakes in the documentation.

### Does this PR introduce _any_ user-facing change?

Yes, this corrects some issues in documentation related to Tuning Spark. The following changes were made
Check if there are too many garbage collections by collecting GC stats. If a full GC is invoked multiple times ~~for~~
  before a task completes, it means that there isn't enough memory available for executing tasks.
with `-XX:G1HeapRegionSize`. (added missing period).
we can estimate the size of Eden to be `4*3*128MiB`. (added the to estimate the size--this one I guess debatable)

### How was this patch tested?

No tests added as this was markdown documentation for the user facing page.

Closes apache#38499 from dwsmith1983/master.

Lead-authored-by: Dustin William Smith <[email protected]>
Co-authored-by: dustin <[email protected]>
Signed-off-by: Sean Owen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants