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

docs: handling "System limit for number of file watchers reached" error #25551

Merged
merged 7 commits into from
Nov 16, 2023

Conversation

nitish-samsung-jha
Copy link
Contributor

Updated docs for handling "System limit for number of file watchers reached" error

Updated document for "System limit for number of file watchers reached" error
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Congrats on making your first PR and thank you for contributing to Superset! 🎉 ❤️

We hope to see you in our Slack community too! Not signed up? Use our Slack App to self-register.

@justinpark justinpark requested a review from sfirke October 6, 2023 16:56
@sfirke
Copy link
Member

sfirke commented Oct 6, 2023

Thanks for this PR. I can review from a language + formatting point of view, but I am unfamiliar with building the assets and would like someone to review this who can evaluate the code itself.

Copy link
Contributor

@mdeshmu mdeshmu left a comment

Choose a reason for hiding this comment

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

I have reviewed it from an operating system perspective. Please make the suggested changes.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@nitish-samsung-jha
Copy link
Contributor Author

I have reviewed it from an operating system perspective. Please make the suggested changes.

Thanks for review. I have updated the docs as per changes suggested.

@nitish-samsung-jha
Copy link
Contributor Author

@mdeshmu could you please review.

@mdeshmu
Copy link
Contributor

mdeshmu commented Oct 18, 2023

@sfirke please review

@nitish-samsung-jha
Copy link
Contributor Author

@sfirke request you to review.

CONTRIBUTING.md Outdated
cat /proc/sys/fs/inotify/max_user_watches
```
Edit the file /etc/sysctl.conf to increase this value.
The value needs to be decided based on the system memory.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The value needs to be decided based on the system memory.
The value needs to be decided based on the system memory [(see this StackOverflow answer for more context)](https://stackoverflow.com/questions/535768/what-is-a-reasonable-amount-of-inotify-watches-with-linux).

CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated
@@ -610,6 +610,32 @@ Then put this:
export NODE_OPTIONS=--no-experimental-fetch
```

If Following type of error comes while running dev server(i.e using above commands):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If Following type of error comes while running dev server(i.e using above commands):
If while using the above commands you encounter an error related to the limit of file watchers:

CONTRIBUTING.md Outdated
Error: ENOSPC: System limit for number of file watchers reached
```
The error is thrown because the number of files monitored by the system has reached the limit.
The error can be fixed by increasing the amount of inotify watchers.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The error can be fixed by increasing the amount of inotify watchers.
You can address this this error by increasing the number of inotify watchers.

CONTRIBUTING.md Outdated
The error is thrown because the number of files monitored by the system has reached the limit.
The error can be fixed by increasing the amount of inotify watchers.

Do the following to Modify the number of system monitoring files :
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Do the following to Modify the number of system monitoring files :

CONTRIBUTING.md Outdated
Do the following to Modify the number of system monitoring files :


The current value of max watches can be checked by below command.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The current value of max watches can be checked by below command.
The current value of max watches can be checked with:

Copy link
Member

Choose a reason for hiding this comment

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

This PR sometimes says "watches" and sometimes "watchers" - is it correct that both terms get used in this context?

CONTRIBUTING.md Outdated
```bash
fs.inotify.max_user_watches=524288
```
save the file and exit
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
save the file and exit
Save the file and exit

CONTRIBUTING.md Show resolved Hide resolved
@sfirke
Copy link
Member

sfirke commented Nov 6, 2023

@nitish-samsung-jha sorry this took me so long to review! I left some requested changes and questions, I think these will just be minor edits I hope. Thank you for contributing this 🙏

@nitish-samsung-jha
Copy link
Contributor Author

@sfirke Thanks for review comments. I have incorporated the suggested changes.
kindly review.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Member

@sfirke sfirke left a comment

Choose a reason for hiding this comment

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

Just two more tiny fixes (sorry if I was the one who put that period in the wrong place 😳 )

Co-authored-by: Sam Firke <sfirke@users.noreply.github.com>
Co-authored-by: Sam Firke <sfirke@users.noreply.github.com>
@nitish-samsung-jha
Copy link
Contributor Author

@sfirke , Thanks for review comments. I have applied suggested changes.
please review

Copy link
Member

@sfirke sfirke left a comment

Choose a reason for hiding this comment

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

LGTM!

@sfirke sfirke merged commit 7f0c3b2 into apache:master Nov 16, 2023
26 checks passed
@sfirke
Copy link
Member

sfirke commented Nov 16, 2023

thanks so much @nitish-samsung-jha !

josedev-union pushed a commit to Ortege-xyz/studio that referenced this pull request Jan 22, 2024
…or (apache#25551)

Co-authored-by: Sam Firke <sfirke@users.noreply.github.com>
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
…or (apache#25551)

Co-authored-by: Sam Firke <sfirke@users.noreply.github.com>
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.1.0 labels Mar 8, 2024
sfirke added a commit to sfirke/superset that referenced this pull request Mar 22, 2024
…or (apache#25551)

Co-authored-by: Sam Firke <sfirke@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/S 🚢 3.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants