Skip to content
Snippets Groups Projects

Add support for saving data to multiple sinks

Merged Karol Krizka requested to merge combsink into devel

Adds a new class called CombinedSink that saves data to multiple sinks. This is done by forwarding all of the necessary calls to instances of given sinks.

This stream also adds a concept of a data stream. A data stream is a combination of IDataSinks targeting a specific set of data. They are defined in a new block called datastreams and returned by EquipConf::getDataStream as a CombinedSink. A monitoring application should use data streams for saving data as they are much more flexible than individual sinks. They allow saving data to multiple and varied combinations of destinations.

The ps_monitor and datasink_example tools have been update to use stream definitions instead of sinks.

An example configuration file looks like this:

{
    "datasinks":{
	"Console":{
            "sinktype": "ConsoleSink"
        },
	"File":{
            "sinktype": "CSVSink",
            "directory": "myOutputData"
        },
	"db": {
            "sinktype": "InfluxDBSink",
            "host": "127.0.0.1",
            "port": 8086,
            "database": "dcsDB",
            "username": "userName",
            "password": "password",
	    "precision": 5
        }
    },
    "datastreams":{
	"PowerSupplies":{
	    "sinks": ["Console", "db"]
	},
	"Climate":{
	    "sinks": ["Console"]
	},
    }
}
Edited by Karol Krizka

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Karol Krizka added 1 commit

    added 1 commit

    • 81681dc5 - Remove unneeded includes in CombinedSink.

    Compare with previous version

  • @epianori @theim @spagan @eressegu @dantrim

    Any thoughts on how this is implemented?

    • Resolved by Simone Pagan Griso

      Without having checked the exact implementation, shouldn't it automatically use a combined sink in case multiple sinks are defined? Hence the definition in the json seems redundant to me, I would have naively assumed the call for combined sink occurs in the background hidden from the user.

      Or is this so the user can choose to redirect some data to some sinks but not others?

  • Timon Heim resolved all threads

    resolved all threads

    • Resolved by Elisabetta Pianori

      Hi @kkrizka,

      I have a question about the json config for dataksink. At the moment we list the available data sinks and their properties in a separate json block. How do we associate data sinks to specific channels?

      Let's say that I want to write to CSV and influxDB data from the low voltage channel, but not to CSV for the Peltier.

      Would it not be better to associate multiple datasinks to an hardware piece?

      Cheers, Elisabetta

    • Resolved by Elisabetta Pianori

      Hi @kkrizka, I think it looks good.

      @theim My first thought is that I think I would disagree that the CombinedSink configuration should be in the background (and not in the JSON configuration). With the way that it is now, the user can configure each individual sink in whatever they wish and then, depending on their situation, essentially turn them on or off by changing only whether or not their name appears in the All.sinks list. In this way, to turn on or off a sink, the user does not need to remove entirely a JSON configuration block but can instead just remove a single string from the All.sinks list. This can then be something that is easier to configure from, e.g. the command line (e.g. $ ./my_program --sinks csv or $ ./my_program --sinks csv,file,influx) which could then override the JSON configuration dynamically. Just a thought.

      So in my mind, I would propose that:

      1. Instead of All it should be Enabled (or something leading to that effect)
      2. The loading of data sinks under the top-level CombinedSink should only enable or call addSink on those DataSinks that are under the currently-named All.sinks list. In this way, the data sink configuration would be required to have the All/Enabled block configuring the always-present CombinedSink.

      Would that make sense?

  • Hi @kkrizka , looks great. I don't have brilliant ideas on how to solve the ad-hoc treatment of the CombinedDataSink in the EquipConf lib unfortunately.. I see that it's not ideal but I guess we'll need to live with it unless someone else has a brilliant idea..

  • Karol Krizka unmarked as a Work In Progress

    unmarked as a Work In Progress

  • Karol Krizka changed the description

    changed the description

  • Karol Krizka added 28 commits

    added 28 commits

    • 81681dc5...c25fcbde - 24 commits from branch devel
    • dc1deab5 - Add CombinedSink.
    • b259dc79 - Undo removal of db example sink.
    • 0879b972 - Remove unneeded includes in CombinedSink.
    • d8dbdf4a - Add datastreams hardware config block that is used by DataSinkConf combined sinks.

    Compare with previous version

  • Karol Krizka added 1 commit

    added 1 commit

    • c10e6cc3 - ps_monitor and datasink_example now use streams.

    Compare with previous version

  • Karol Krizka added 1 commit

    added 1 commit

    Compare with previous version

  • Karol Krizka changed the description

    changed the description

  • @epianori @theim @spagan @eressegu @dantrim I've updated the MR based on what we've discussed at the labRemote meeting. The concept is still the same, the CombinedSink is still created by a special function inside DataSinkConf class.

    The difference is that it is now created by DataSinkConf::getDataStream instead of as a special case of DataSinkConf::getDataSink. The CombinedSink's are also now defined in a new datastreams block inside the JSON file to conceptually separate their special use case.

    From my end, this MR is now ready. Just need to fix the Python bindings, which will be done in the next minute or so.

    Edited by Karol Krizka
  • Karol Krizka added 11 commits

    added 11 commits

    • 00be68ae...978841ee - 4 commits from branch devel
    • c636beb5 - Add CombinedSink.
    • a328f2d3 - Undo removal of db example sink.
    • 8f2f798b - Remove unneeded includes in CombinedSink.
    • 52fc89c9 - Add datastreams hardware config block that is used by DataSinkConf combined sinks.
    • 2dab18bc - ps_monitor and datasink_example now use streams.
    • 34c7f045 - Documentation updates.
    • 0331101c - Fix python bindings for libDataSink.

    Compare with previous version

  • Hi @kkrizka ,

    I think this makes a ton of sense and I like it way better now that the concept of data stream is separated from the data sink (but correctly inheriting still from IDataSink).

    I went through the code and I don't have any significant comment, so I'll approve it.

    Thanks!

    Simone

  • Simone Pagan Griso approved this merge request

    approved this merge request

  • @epianori Can this be merged?

    The only unresolved threads were discussions to prompt the in-person chat during the Wednesday meeting.

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
Please register or sign in to reply
Loading