Skip to content

Conversation

@joefowler
Copy link
Member

Not sure you'll be excited to add an optional DB-logging feature, but I do believe that it's feature-complete for a certain first set of tasks. I made enough changes that I'd like to bake this into the main branch. It shouldn't have any effect on computers where DB use was not explicitly asked for in the past.

This PR does not log pulse data, but only:

  1. A row of info about this current invocation of the Dastard process (Go version, git hash, process start time and eventually, its end time)
  2. A row of info about each data run (intention, source type, directory path, # of channels, timebase, number of samples and presamples)
  3. A row of info about each file (filename, file type, start time, and after it's closed: the end time, record count, file size, and a sha256 hash of the file).

Again, there are no pulse records stored in the DB--no LJH-like or OFF-like data. I experimented with that possibility, but the performance was not acceptable. (See branch:clickhouse_pulse_storage for that code.)

@joefowler joefowler requested a review from ggggggggg November 18, 2025 22:04
@ggggggggg
Copy link
Collaborator

What benefit do we gain by merging this vs just keeping it in a branch/PR until we actually want to test the features?

@joefowler
Copy link
Member Author

What benefit do we gain by merging this vs just keeping it in a branch/PR until we actually want to test the features?

I am not sure. My motivation for this PR is simply that both this work and the possible work on simplifying our data writing touch/would touch the same lines of Go. If we're going to change how we write LJH and OFF files, I don't want to make incompatible changes for that purpose and for the DB writer. I'd rather launch the one branch from the other.

Of course, that doesn't require us to merge this PR yet. We can talk in person....

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

Successfully merging this pull request may close these issues.

3 participants