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

Hasura log size #11

Closed
hongbo-miao opened this issue Apr 28, 2022 · 6 comments
Closed

Hasura log size #11

hongbo-miao opened this issue Apr 28, 2022 · 6 comments

Comments

@hongbo-miao
Copy link
Contributor

hongbo-miao commented Apr 28, 2022

Hi @afitzek if I understand correctly, based on this approach by using

command: ["/bin/sh", "-c", ": > /tmp/log/stdout.log && /bin/graphql-engine serve | tee /tmp/log/stdout.log"]

The /tmp/log/stdout.log file will become bigger and bigger until the Hasura container dies or redeploys.

Need somehow limit the log size.

@hongbo-miao
Copy link
Contributor Author

hongbo-miao commented Apr 28, 2022

I found this https://unix.stackexchange.com/questions/17209/how-to-limit-log-file-size-using which might help!

Or control the size by hasura-metric-adapter code? 😃

@afitzek
Copy link
Owner

afitzek commented Apr 29, 2022

Hi @hongbo-miao,

I wasn't really happy with the current solution, the first thing that comes to my mind is to use a named pipe. The containers in the pod need to share a process namespace in this case.

So in the spec of the deployment we need shareProcessNamespace: true

the command should than can become:

command: ["/bin/sh", "-c", "rm -rf /tmp/log/stdout.log && mkfifo /tmp/log/stdout.log && /bin/graphql-engine serve | tee /tmp/log/stdout.log"]

The file size should there be 0, since the actual log data is only hold in a shared memory in the named pipe.

The disadvantage of this is, that if only the metric adapter dies for whatever reason, it can't recover the correct metrics for open websocket connections etc. because it can't replay the log.

Let me know what you think. :)

@hongbo-miao
Copy link
Contributor Author

hongbo-miao commented Apr 29, 2022

I am not an expert, however I feel better than current approach. 😊
Because for the current approach, the log file will easily become huge causing the container die when having a lot of traffic.

@afitzek
Copy link
Owner

afitzek commented Apr 29, 2022

@hongbo-miao I created the PR #12 to explain the named pipe approach in the Readme, the adapter doesn't need any adaption to work with a named pipe. :)

@afitzek
Copy link
Owner

afitzek commented Apr 29, 2022

Thanks for bringing this up in the first place! 😄

@hongbo-miao
Copy link
Contributor Author

hongbo-miao commented Apr 29, 2022

I just tried, works perfect! 😄
Both old and new approach, when reploy, the metrics will reset.
If understand correctly, the new approach will have add a new downside only when metric adapter dies. I think as long as this adapter is stable in futre, it will rarely happen.
The the log file won't become bigger and bigger for new approach, which I think it is a big gain : )

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

No branches or pull requests

2 participants