Large postgres logs #105
Large postgres logs #105
Conversation
Large Postgres files may pose a challenge for some systems. This change stores the file on disk, compares the filesize with the remote, and adds the local file to the tar. It has error handling to allow a support export to be generated when there are issues with gathering PG logs.
Create a unique sudirectory in outputDir (matching the unique timestamp) so that local files are logically separate from other files. Adding messaging and error handling so a user understands.
internal/cmd/export.go
Outdated
| req := clientset.CoreV1().RESTClient(). | ||
| Get(). | ||
| Resource("pods"). | ||
| Name(podName). | ||
| Namespace(namespace). | ||
| SubResource("exec"). | ||
| Param("container", containerName). | ||
| Param("command", "cat"). | ||
| Param("command", remotePath). | ||
| Param("stderr", "true"). | ||
| Param("stdout", "true") | ||
|
|
||
| exec, err := remotecommand.NewSPDYExecutor(config, "GET", req.URL()) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to initialize SPDY executor: %w", err) | ||
| } |
There was a problem hiding this comment.
Could we use the util.NewPodExecutor to set this command up?
There was a problem hiding this comment.
The problem I've had there is the current flow assumes the contents will be held in memory. I couldn't figure out how to use the current flow to pass to *os.File - Definitely a limitation on my part, so feedback is welcome
There was a problem hiding this comment.
hmmm, NewPodExecutor returns a func that takes stdout, stderr io.Writer, and I'm not against writing a specific function for this executor, like
func (exec Executor) copyFile(source string, destination *os.File) (string, error) {
var stderr bytes.Buffer
command := fmt.Sprintf("cat %s", source)
err := exec(nil, &desination, &stderr, "bash", "-ceu", "--", command)
return stderr.String(), err
}
That's a slight variation on the catFile func, the main difference being that it sets the output to the file (I think) and doesn't return a stdout. Not sure that would work, but that's my first thought.
There was a problem hiding this comment.
one slight change &destination to destination, but I think this works.
func (exec Executor) copyFile(source string, destination *os.File) (string, error) {
var stderr bytes.Buffer
command := fmt.Sprintf("cat %s", source)
err := exec(nil, destination, &stderr, "bash", "-ceu", "--", command)
return stderr.String(), err
}
internal/cmd/export.go
Outdated
| req := clientset. | ||
| CoreV1(). | ||
| RESTClient(). | ||
| Post(). | ||
| Namespace(namespace). | ||
| Resource("pods"). | ||
| Name(podName). | ||
| SubResource("exec"). | ||
| Param("container", containerName). | ||
| Param("command", "stat"). | ||
| Param("command", "-c"). | ||
| Param("command", "%s"). | ||
| Param("command", filePath). | ||
| Param("stderr", "true"). | ||
| Param("stdout", "true") | ||
|
|
||
| exec, err := remotecommand.NewSPDYExecutor(config, "GET", req.URL()) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("could not initialize SPDY executor for stat: %w", err) | ||
| } |
There was a problem hiding this comment.
Ditto question re: util.NewPodExecutor
There was a problem hiding this comment.
I refactored this one to use the bashCommand function and executor. This one is easier because it's a simple stat command.
internal/cmd/export.go
Outdated
| } | ||
|
|
||
| // ConvertBytes converts a byte size (int64) into a human-readable format. | ||
| func ConvertBytes(bytes int64) string { |
There was a problem hiding this comment.
We're not exporting this command for another package to use, so convertBytes should be fine. (Also here's a case where I think tests would do some work as documentation.)
There was a problem hiding this comment.
I renamed this one
| } | ||
|
|
||
| // compare file sizes | ||
| localFileInfo, err := os.Stat(localPath) |
There was a problem hiding this comment.
Here we use os.Stat(FILE) and on line 1912 we use file.Stat() -- they're functionally the same, right? Not a blocker, just curious if we could use the same func twice. (I'm looking at the code for the two functions and they're not identical in the way I sort of thought they might be, though they both end up calling the fillFileStatFromSys helper func.)
gathering large Postgres files may pose a challenge for some systems. This change stores the file on disk, compares the filesize with the remote, and adds the local file to the tar. It has error handling to allow a support export to be generated when there are issues with gathering PG logs.