Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change StorageDevice to contain a data path and disk quota #40

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

eflumerf
Copy link
Member

No description provided.

@eflumerf
Copy link
Member Author

Ensemble with
DUNE-DAQ/dfmodules#378
#40
DUNE-DAQ/appmodel#115

@glehmannmiotto
Copy link
Contributor

added @gcrone as reviewer since he originally introduced the storage device class

Copy link
Contributor

@gcrone gcrone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original intent was that the StorageDevice class describes the physical hardware available on the machine. Should the quota and data_path be in a separate class defining how the StoarageDevice is used by the software?

@eflumerf
Copy link
Member Author

My thinking here was that systems could have multiple StorageDevices (e.g. /data1, /data2, ...), and the only reason we would care about their size is if we wanted to impose a limit on how much of it a given Session could use at a time. If you look at the associated dfmodules, PR, you can see that this interpretation helps with setting up DataWriters to target specific StorageDevices

@gcrone
Copy link
Contributor

gcrone commented Sep 10, 2024

I can see that the idea of quota and the actual data path are things that you want in the database. The point was that the StorageDevice class was meant to represent the actual hardware associated with a host and be used for purposes of CPU pinning etc rather than representing an aspect of the application that was using the device. I know we haven't made any progress on the hardware description / pinning aspects yet but it seems sensible to keep the 2 separate.

Copy link
Contributor

@gcrone gcrone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK

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