generalTime provider needs API to supply userTag

Bug #1736911 reported by Bruce Hill
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
EPICS Base
New
Wishlist
Unassigned

Bug Description

I noticed when reviewing Marty's recent pull request for pvDatabaseCPP that his patch was needed because it relies on the generalTime provider which doesn't support userTags.
   https://github.com/epics-base/pvDatabaseCPP/pull/31

A generalTime provider that supports userTags will be needed for any facilities which set their userTag using a pulseId from the timing stream.

I'm thinking the current API could stay as is, but a new set of API functions could be added that would support both an epicsTimeStamp and a userTag. Tnen an eventUserTagProviders list could be checked first to see if a userTag provider was available.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

My $0.02 is to add a member to struct epicsTimeStamp rather than add a whole new API.

Changed in epics-base:
importance: Undecided → Wishlist
Revision history for this message
Andrew Johnson (anj) wrote :

Modifying epicsTimeStamp would require splitting it into 2 separate definitions or the result would break the CA API and wire protocol. All of the dbr_time_* structures in db_access.h use an epicsTimeStamp, so every existing CA client binary is expecting the current definition and would break if is changed.

Unfortunately I now think the change I advocated in Bug #1722530 can probably only be a pipe-dream, so a new type and API are probably needed.

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

Well damn, db_access.h could I think be worked through. However, struct epicsTimeStamp is also used by the channelarchiver file (de)serialization code. So a change would break archiver file compatibility.

https://github.com/epicsdeb/channelarchiver/blob/2b41c359b2f933b2807842f74d6940e0d0c582ce/LibIO/BinChannel.h#L73

https://github.com/epicsdeb/channelarchiver/blob/2b41c359b2f933b2807842f74d6940e0d0c582ce/LibIO/BinChannel.cpp#L62

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

The complication with a new API is the (significant) work need to teach frameworks like eg. asyn how to handle it.

Revision history for this message
Bruce Hill (bhill) wrote :

I like the idea of having a userTag in pvAccess, the question is how do we make use of it?
Currently, it's up to EPICS engineers to make sure the userTag gets updated when needed.
Even if we make our drivers V4 aware, it only allows setting userTags in those specific V4
objects. It doesn't allow modules like pvaSrv or pva2pva to set the userTag appropriately
when publishing dbRecord's via pvAccess.

Most of the approaches we've been considering involve trying to add userTags to the db and
propagate them via TSEL or TSE mechanisms. What if we stop trying to add a userTag to the
record database since we can't easily add it to CA, and instead just provide a way for site's
to determine how pvData userTag's get set and propagate.

What I like about adding a userTagProvider() API is that it wouldn't affect modules like asyn that just interface w/ the dbCore API. Since the userTag was added in pvData and the other V4 modules, we just need to make sure those modules call the userTagProvider() whenever they update the pvData::TimeStamp.

As to the API, SLAC would just need the timeStamp to derive the userTag, but I think we could
provide an API that would provide some options for other userTag use cases.

We'd also be ok w/ a signed 64 bit userId as we could easily wrap it to unsigned,
and I think a signed integer would provide more options for site specific userTags.

How about this for the API of a userTag provider callback?
Caller can pass NULL for any parameters which aren't available and the
userTag provider can determine the policy for how to set the userTag given the available info.

typedef epicsInt64 (* USERTAGFUN)( const epicsTimeStamp * pTimeStamp, const dbCommon * pRec, const epicsInt64 * pUserTag );

Revision history for this message
mdavidsaver (mdavidsaver) wrote :

> What I like about adding a userTagProvider() API is that it wouldn't affect modules like asyn

To my mind, this sounds like something which can be added to eg. QSRV directly without
bothering with any change to Base.

I would caution that doing this lookaside blindly for all records could easily introduce another point of coupling (via a mutex) between high and low priority scan threads. This is something SLAC has been concerned about in the past.

Revision history for this message
Andrew Johnson (anj) wrote :

@bhill I'm sure you're aware that the pvAccess userTag is current defined as an int32 in pv/timeStamp.h, so you'll also need to propose that it be widened to make it 64-bit (I would support making it unsigned as well if we do decide to change it). This will have knock-on effects and require changes to other modules which may trigger some push-back, so if that's your plan please create an issue (on Github) against both the pvDataCPP and pvDataJava modules to start that discussion.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.