On Tue, Aug 31, 2021 at 8:43 PM Michael Paquier <mich...@paquier.xyz> wrote:
> On Tue, Aug 31, 2021 at 11:34:56AM -0400, Sehrope Sarkuni wrote: > > The second commit adds a TAP test for log_destination "csvlog". This was > > done to both confirm that the previous change didn't break anything and > as > > a skeleton for the test in the next commit. > > +note "Before sleep"; > +usleep(100_000); > +note "Before rotate"; > +$node->logrotate(); > +note "After rotate"; > +usleep(100_000); > > Do you really need a rotation of the log files here? Wouldn't it be > better to grab the position of the current log file with a fixed log > file name, and then slurp the file from this position with your > expected output? That would make the test faster, as well. > Yes, that was intentional. I used the log rotation tap test as a base and kept that piece in there so it verifies that the csv log files are actually rotated. Same for the json logs. > Rather than making elog.c larger, I think that we should try to split > that into more files. Why not refactoring out the CSV part first? > You could just call that csvlog.c, then create a new jsonlog.c for the > meat of the patch. > That's a good idea. I'll try that out. The list of fields is not up to date. At quick glance, you are > missing: > - backend type. > - leader PID. > - query ID. > - Session start timestamp (?) > Thanks. I'll take a look. Regards, -- Sehrope Sarkuni Founder & CEO | JackDB, Inc. | https://www.jackdb.com/
