deprecate pvCopy.h?

[mdavidsaver]

I'm considering deprecating it in pvDataCPP for removal after 8.0.0. As usage of pvCopy.h seems to be limited to pvDatabaseCPP, perhaps this code should be migrated into pvDatabaseCPP? I have no immediately pressing reason to do so. This is more about reducing public API size in the longer term.

I've never been clear on what problem(s) this code is trying to solve. I have the impression that it involves a situation which I avoided in QSRV by giving every client the exact same Structure.

@mrkraimer Would you like to make a case for keeping this code in pvDataCPP?

[mrkraimer]

The goals for pvData/pvAccess going back to 2006 (javaIOC) include:

1. Define and implement client/server support for structured data.
2. A server provides data via a PVStructure.
3. A client can request data from an arbitrary subset of the fields in the PVStructure.
4. For gets and monitors the server can send data only for the fields that have been modified since the last get or monitor. The client can find out which fields were modified.
5. An arbitrary number of channel providers can be implemented.

pvRequest is a facility that makes it easy for clients to implement 3).
It also defines a standard for how 2) is communicated from the client to the server.
pvCopy is a facility that can be used by channel providers to implement 3) and 4).

At the present time the following providers are implemented:

a) The local provider in pvDatabase. It implements 2), 3), and 4).
It uses pvCopy to implement 3) and 4).

b) ca provider (in pvAccess). This implements 2) and 3). It should be changed to also support 4).
NOTE: because of the dbd definitions, It may not be able to implement a complete and efficient implementation of 3). Especially for monitors.

c) qsrv (in pva2pva). This implements 2). It should be changed to support 3) and 4).
NOTE: it may not be able to implement a complete and efficent implementation of 3).

I want to see how ca provider can implement 4). I think that pvCopy may help.
Note however that extra data is always sent over the network.
Thus only qsrv could implement this efficiently.

mdavidsaver said

I've never been clear on what problem(s) this code is trying to solve. I have the impression that it involves a situation which I avoided in QSRV by giving every client the exact same Structure.

Note that this means that 3) is not satisfied.
Perhaps the above goal will help explain the problem pvCopy is trying to solve.

[mdavidsaver]

c) qsrv (in pva2pva). This implements 2). It should be changed to support 3) and 4).
NOTE: it may not be able to implement a complete and efficent implementation of 3).

I will agree with your 4) however I see no benefit, and at least one drawback, to your 3).

I've opened epics-base/pva2pva#11 as a reminder to add masking of monitor updates via pvRequest (4).

wrt. 3) I want to avoid a proliferation of types (Field) as this burdens the caching in PVA. Mainly though, I find it confusing that different users can see different structures.

[mdavidsaver]

Perhaps the above goal will help explain the problem pvCopy is trying to solve.

It does, though I still find the API confusing. However, I'll refrain from deprecating it until I've closed epics-base/pva2pva#11. If I don't find a place for pvCopy in doing this, then I'll probably deprecate it.

[mrkraimer]

The following is not what a client expects when accessing a DBRecord

mrk> pvget -m -r "value,alarm,timeStamp" DBRdouble00
DBRdouble00
epics:nt/NTScalar:1.0
double value 0
alarm_t alarm INVALID DRIVER UDF
time_t timeStamp 0
display_t display
double limitLow 0
double limitHigh 0
string description
string format
string units
control_t control
double limitLow 0
double limitHigh 0
double minStep 0
valueAlarm_t valueAlarm
boolean active false
double lowAlarmLimit nan
double lowWarningLimit nan
double highWarningLimit nan
double highAlarmLimit nan
int lowAlarmSeverity 0
int lowWarningSeverity 0
int highWarningSeverity 0
int highAlarmSeverity 0
double hysteresis 0

DBRdouble00
epics:nt/NTScalar:1.0
double value 1
alarm_t alarm NO_ALARM NO_STATUS NO_ALARM
time_t timeStamp 2018-02-07T06:22:40.309 0
display_t display
double limitLow 0
double limitHigh 0
string description
string format
string units
control_t control
double limitLow 0
double limitHigh 0
double minStep 0
valueAlarm_t valueAlarm
boolean active false
double lowAlarmLimit nan
double lowWarningLimit nan
double highWarningLimit nan
double highAlarmLimit nan
int lowAlarmSeverity 0
int lowWarningSeverity 0
int highWarningSeverity 0
int highAlarmSeverity 0
double hysteresis 0

[mrkraimer]

Note that using provider ca produces:
Cmrk> pvget -m -r "value,alarm,timeStamp" -p ca DBRdouble00
DBRdouble00
epics:nt/NTScalar:1.0
double value 1
alarm_t alarm NO_ALARM NO_STATUS NO_ALARM
time_t timeStamp 2018-02-07T06:22:40.309 0

DBRdouble00
epics:nt/NTScalar:1.0
double value 2
alarm_t alarm NO_ALARM NO_STATUS NO_ALARM
time_t timeStamp 2018-02-07T06:25:15.965 0

[anjohnson]

@mrkraimer 's goal number 3 is needed to support clients like alarm handlers, which have no interest in a PV's value at all, and may also be essential when talking to PVs that have large arrays in some of their fields, especially when the client or IOC is bandwidth-limited and the client doesn't want to see the array data. I commented in epics-base/pva2pva#11 that the current pva2pva behavior is a regression from pvaSrv.

[mdavidsaver]

I fell I'm obligated to point out that to my knowledge, no actual users are specifying pvRequest. There is certainly no "alarm handler".

[anjohnson]

Sorry Michael, but you are not omniscient. We have been advertising the "fetch only a subset of the fields" feature to our scientists here at APS, which works on Siniša's latest tool for serving data from an SDDS file over PVA. That tool is based on top of pvDatabaseCPP which implements it using pvCopy.

[mdavidsaver]

Does this mean that APS has actual user composed pvRequest strings? Something other than 'field()' or 'field(value)'? Can you share some examples?

[mrkraimer]

https://github.com/caqtdm/caqtdm/blob/Development/caQtDM_Lib/caQtDM_Plugins/epics4/epics4_plugin.cpp

Has the statements:

void PVAInterface::createMonitor()
{
if(Epics4Plugin::getDebug()) cout << "PVAInterface::createMonitor()\n";
try {
if(normativeType==ntunknown_t) return;
string request("value,alarm,timeStamp");

[anjohnson]

I can't show you any embedded in our code, but the pvRequest string is an optional argument to both pvget and eget and as such it's a very public and documented API.

[anjohnson]

Siniša has this on one of the slides he was showing on Tuesday:

ctlsdaqdev1> pvget -r 'field(S35DCCT_currentCC)'  fft_bm
fft_bm
structure
    double S35DCCT_currentCC 99.2111

That PV fft_bm was served from an SDDS file that was taken by our DAQ system, which evidently adds secondary information such as the storage-ring current as SDDS attributes (which become scalar fields) alongside the (large) data arrays in the file.

[mrkraimer]

I have been working on ca provider so that for gets and monitors the client can find out which fields have changed value. I realized that PVCopy needs a new public method "updateMasterSetBitSet".
With this addition PVCopy does most of the work.
I created a pull request for this change.

[mdavidsaver]

pvget -r 'field(S35DCCT_currentCC)' fft_bm

This field name makes me cringe, but your point is made.

[gregoryraymondwhite]

Hi everyone, Right. I was also thinking about use cases where the pvStructure had a large payload field but the user only was interested in a low payload field. Eg NTNDArray, a client may only want the codec, not the image. But my opinion on this was reversed after a conversation with Michael. He pointed out to me that a bespoke client can simply choose not to code the introspection interface of fields it’s not interested in. So that leaves standard clients like eget. It’s true, in his proposal they’d see 0s (the defaults) in fields the client didn’t express interest in. Though not brilliant, that certainly seems acceptable to me. If end users really want that they don’t see fields they are not interested, then maybe we can give some awk examples to show how to filter only what they do want. Greg

…

On Feb 8, 2018, at 1:01 PM, Andrew Johnson ***@***.***> wrote: Siniša has this on one of the slides he was showing on Tuesday: ctlsdaqdev1> pvget -r 'field(S35DCCT_currentCC)' fft_bm fft_bm structure double S35DCCT_currentCC 99.2111 That PV fft_bm was served from an SDDS file that was taken by our DAQ system, which evidently adds secondary information such as the storage-ring current as SDDS attributes (which become scalar fields) alongside the (large) data arrays in the file. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[mdavidsaver]

wrt. eget. I don't intend this to apply to the rpc operation (though it could). My goal is that, for the sake of user sanity, that get, put, and monitor should work with exactly the structure returned by Channel::getField().

As only one Structure is involved, the BitSet masked overload of PVStructure::copyUnchecked() can be used instead of pvCopy. The change I intend to make for epics-base/pva2pva#11 would simply be to how this mask is computed. This is a lot simpler for a provider to implement, and probably marginally faster.

[mdavidsaver]

related epics-base/pvAccessCPP#41

[mrkraimer]

The goals for pvData/pvAccess going back to 2006 (javaIOC) include:

3. A client can request data from an arbitrary subset of the fields in the PVStructure.

Since qsrv returns the same Structure to all clients, this goal is not satisfied.

[ralphlange]

Sure.
But since this is a goal for pvData/pvAccess, shouldn't pvData/pvAccess take care of that?

Here's how I see it: A server application (QSRV on top of an IOC Database) has an internal structure that it wants to expose. To do that over pvAccess, it registers as a channel provider.
Why is a promise that the pvAccess server makes to its clients (that they can request a subset of the complete structure) in the scope of QSRV? Shouldn't that be the job of pvAccess?

[gregoryraymondwhite]

On Feb 15, 2018, at 4:50 AM, Ralph Lange ***@***.***> wrote: Sure. But since this is a goal for pvData/pvAccess, shouldn't pvData/pvAccess take care of that? Here's how I see it: A server application (QSRV on top of an IOC Database) has an internal structure that it wants to expose. To do that over pvAccess, it registers as a channel provider. Why is a promise that the pvAccess server makes to its clients (that they can request a subset of the complete structure) in the scope of QSRV? Shouldn't that be the job of pvAccess?

Well, that’s a very good point.

…

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[mrkraimer]

Ralph asks: "Shouldn't that be the job of pvAccess?"

But pvAccess is just the name of a set of software that can be used to implement
client and server code. pvAccess.

pvAccess provides the following:

1. The client side of the pva network provider. It communicates with the server side of pva.
2. The ca channel provider. This uses the standard ca network protocol.
3. The sever side of pva. This allows an arbitrary number number of server side providers. The server side of pva is basically a client that accesses channels implemented by server side providers.
4. A client and server registry for providers.

Two server side providers are currently implemented

1. local provider which is implemented in pvDatabase and provides access to PVRecords.
2. pva2pva implements qsrv (The former pvaSrv) which provides access to DBRecords.

A provider must implement classes Provider and Channel.
The most important method of Provider is createChannel.
This method, given a channelName, creates a Channel.
A Channel has methods: createChannelGet, createChannelPut, etc.
The provider can implement each create method or report failure.
Each create method has a pvRequest argument.
One of the things the pvRequest specifies is the subset of the fields the client desires.

The code in pvCopy can be used by a channel provider to provide access to a subset of the fields in a top level PVStructure associated with a channel.
It is used by provider local and, with code that I can not push until pvDataCPP pull request 53 is merged, by the ca provider.

[anjohnson]

The original "subset of fields" goal still needs be implemented somewhere; where and how is something we could change if we really wanted to, although it seems rather late to be making what appears to be an architectural change after the building is already occupied and in daily use.

Currently pvget and eget allow the user to give a list of the fields they want in the pvRequest specification, and only the data in those fields will be transported and displayed. Both pvaSrv and pvDatabase implement this behavior, and all PVA client interfaces let the user provide a pvRequest specification. Marty is currently adding support for this to the caProvider, and we have published documentation that explains how to use all these interfaces.

Marty just described how different layers of the current design are responsible for different functionality, and how the server really just implements a remoting capability for the individual providers. Moving the field subset functionality out of the individual providers into the server layer would presumably also remove the ability to request a field subset from a local provider, which would appear to be a problem with this change.

If we did agree to change how a user can fetch a subset of the available fields, I want to see that change discussed properly and any new design agreed to first, not just implemented by fiat. If the result can use less code and potentially run faster that's great, but network connections between old and new implementations have to work seamlessly (both new client → old server and old client → new server) as IIRC we said we wouldn't break network interoperability after 7.0. Thus the design of any change is probably rather harder than it first seems, and I think it unlikely that we could reduce the amount of code needed once that interoperability is accounted for as well.

I understand that a bespoke client could be coded to only include those fields it wants to see in the pvStructure that it passes to be got/monitored, but what about the generic clients? Repeating my earlier example, the PV fft_bm contains several large array fields that I don't want to fetch or see:

pvget -r 'field(S35DCCT_currentCC)' fft_bm

BTW the V4 website contains many versions of the network protocol document, the latest of which is dated 16th October 2015; is this being kept up to date? I regard the importance of maintaining this document (or a replacement for it) as pretty high.

[mdavidsaver]

When epics-base/pva2pva#11 is accomplished, only the requested field values will be moved over the wire. When further combined with changes to pvget, only changed fields will be printed. As far as I am concerned, the combination satisfies any practical definition of "subset of fields".

[mdavidsaver]

shouldn't pvData/pvAccess take care of that?

Yes. Hell yes! If Marty's pvRequest goals are ever to be universal, then they must been built into the PVA server. The provider interfaces are just too damned complicated. No one seems to be able to avoid violating the myriad locking and ownership rules (which I've tried to document), let alone the semi-documented functional "goals".

Before anyone rushes to start adding this Structure mangling into the PVA server I must offer a word of caution. I've spent the past 9 months chasing bugs out of this part of the code base. This has already resulted in my introducing regressions! For such a major change to be accepted, especially given my dislike of the present pvCopy API, the burden of code quality and testing will be high (there will be good unit test coverage).

[mdavidsaver]

Marty is currently adding support for this to the caProvider

Marty is of course free to spend his time as he sees fit. And you (Andrew) as maintainer of caProvider may accept what you will. I can only wish that the lock order design flaw (epics-base/pvAccessCPP#77) were prioritized over this imo. minor point.

[mdavidsaver]

fyi. I recently added extractRequestMask() in pv/createRequest.h in order to implement the behavior described in epics-base/pva2pva#11 .

    PVStructure::const_shared_pointer value(...), // some Structure with .value
                                      pvRequest(createRequest("field(value)"));
    BitSet fieldMask(extractRequestMask(value, pvRequest->getSubField<PVStructure>("field"));
    assert(fieldMask == BitSet().set(value->getSubFieldT("value")->getFieldOffset());

I'm already making use of this in the SharedPV implementation (cf. "pva/sharedstate.h") for Get and Monitor. When I next have time I'll be changing QSRV to use SharedPV.

[mdavidsaver]

After the addition of PVRequestMapper, I've now deprecated PVCopy (f0fa8a2).

[mdavidsaver]

Released with Base 7.0.2