generalTime provider needs API to supply userTag

[Bruce Hill]

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.

[mdavidsaver (mdavidsaver)]

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

[Andrew Johnson (anj)]

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.

[mdavidsaver (mdavidsaver)]

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

[mdavidsaver (mdavidsaver)]

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

[Bruce Hill (bhill)]

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 );

[mdavidsaver (mdavidsaver)]

> 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.

[Andrew Johnson (anj)]

@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.