afitzek/hasura-metric-adapter

Hasura log size

hongbo-miao opened this issue · 6 comments

hongbo-miao commented

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 commented

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 commented

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 commented

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 commented

@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. :)

  • ❤️1
afitzek commented

Thanks for bringing this up in the first place! 😄

  • 😄1
hongbo-miao commented

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 : )