Repository navigation
Hi. I want to ignore a participant. #78
Description
Activity
Hi @qqqshengwei, in my reading of the specification, the specification doesn't have any conception of something larger than a participant and therefore all participants are 100% independent of each other. (Maybe I'm confusing you here — sorry! — just trying to explain how I look at this because I think it is relevant. The "larger" thing in Cyclone is the idea that you can have a single DDSI stack that handles multiple participants simultaneously while doing discovery only once.)
In the spec's conception of this, it is therefore a very simple operation: when you call the
ignoreoperation, you do a bunch of things:- you add the (remote) participant GUID to some ignore list in the participant;
- you delete the proxy for the remote participant, which automatically gets rid of all readers/writers;
- you ignore any SPDP message that arrives from a participant in the list.
I think you are also allowed to ignore yourself. The above would work even in that case if you implement communication between readers and writers in a single participant on top of the network stack (there are some decent arguments for this and Cyclone has done it that way for some time — that was before the code was opened up — but no longer does so because I don't think the trade-off is the best one. That detail even came up in an earlier discussion: Userdata Qos usecase #54 (comment).)
I think the irreversibility of the operation is either to give implementors more freedom, or it is caused by the use of an instance handle for the remote participant instead of a GUID. The spec doesn't guarantee that instance handles can't be reused, and so after ignoring the participant there is no longer a way to refer to it anymore. Clearly, in the above implementation, undoing it would be trivial if the GUID is known: just remove the GUID from the ignore list and you're done.
You would only run into one problem, and that is that the participant being ignored doesn't "know" it is being ignored, and so it would expect responses from the readers and writers that it had already discovered. Cyclone is not really good at handling that kind of problem at the moment ... (It is actually one of the easier things to do deal with, because it means deciding that some remote entity has lost liveliness if it hasn't responded for too long.)
But ... Cyclone doesn't do local communications via the network and is conceived to handle multiple participants simultaneously ...
The easy option, and a quite reasonable one at that, is to not care about the multiple participants bit, and have a "global" ignore list. In that case the above works for remote participants. For local readers/writers you would have to check whether a match is allowed for new entities (
connect_writer_with_reader) and for the initial dropping of the connections you'd have to run over them and do something along the lines of the loop ingc_delete_reader.That said: I have a question in return. There is this old issue: #8, which is specifically about filtering out the data the participant sent itself. It goes without saying that filtering after reading is rather inefficient ... but the direction I hinted at there of doing content filtering on the writer GUID and then pushing this content filter as far to the writer as possible very much has my preference.
In the case of a content filter on the writer GUID, I would then drop the link from the writer to the reader doesn't want the data from that writer, and try to propagate that knowledge all the way to writer so that it can stop sending data if no other readers are left. That way, there is no crazy asymmetry in the discovery and no need for A to assume B has started ignoring A based on timeouts. I think that is more complicated, but also more elegant, approach.
So the return question is: are you asking because you want to ignore your own data? If that is all you want to achieve, then the content filter bit would actually not be so hard to do, as it could be done set at creation time of the readers and just be taken into account in the initial matching. Pretty much like any other RxO QoS.
Thank you for your generous answer. I found the useful method
participant_instance_idto get a participant handle by its GUID. However, I am confused about how to get GUID of a participant by its instance handle. It is really a good idea to create a const "global" ignore list (I think it should stay inentity.c) and a guard method likeis_ignored_participant_guidinhandle_SPDP_alive. I apologize for my late reply.There is the function
dds_get_instance_handleto get the instance handle for an entity, it forwards toparticipant_instance_id. You could use that instance handle, pass it to thedds_read_instancefunction in a data reader for the DCPSParticipant topic, and you'd end up with a sample that describes the participant, and the key value in this sample is the GUID.There is no really easy way to do the above because there is no hash table that maps entity ids to entities (only hash tables that map GUIDs to entity ids ...). You could create a function to do the lookup and leverage the new-ish built-in topic support. You'd basically do something very similar to the current
dds_instance_get_key. (That is not to say that the code fordds_instance_get_keyis anywhere near correct, because it should inspect the history cache of the reader/writer that's passed in and look in there instead of the current code that can cause all kinds of trouble ...)Basically you'd end up with something like:
dds_get_entity_guid (dds_builtintopic_guid_t *out_guid, dds_instance_handle_t ih) { ret if ((tk = ddsi_tkmap_find_by_id(gv.m_tkmap, ih)) == NULL) { ret = NOT_FOUND; } else { if (tk->m_sample->ops != &ddsi_serdata_ops_builtin) { ret = NOT_FOUND; /* not associated with a built-in topic */ } else { struct ddsi_serdata_builtin *d = tk->m_sample; nn_guid_t tmp = nn_hton_guid (d->guid); memcpy (out_guid, &tmp, sizeof (tmp)); ret = OK; } ddsi_tkmap_instance_unref (tk); } return ret; }
Regarding the ignore list and adding a
is_ignored_participant_guidwould be a sensible step to begin with. Least effort to get the result you need and that without causing trouble in the future 😁.Happy New Year.
I realizeddds_get_entity_guidindds_entity.cwith something like:dds_return_t dds_get_entity_guid ( _Out_ dds_builtintopic_guid_t *guid, _In_ dds_instance_handle_t *ihdl) { dds_return_t ret; struct thread_state1 * const thr = lookup_thread_state(); const bool asleep = !vtime_awake_p(thr->vtime); struct ddsi_tkmap_instance *tk; if (asleep) { thread_state_awake(thr); } if ((tk = ddsi_tkmap_find_by_id(gv.m_tkmap, ihdl)) == NULL) { DDS_ERROR("No instance related with the provided handle is found\n"); ret = DDS_ERRNO(DDS_RETCODE_PRECONDITION_NOT_MET); } else { if(tk->m_sample->ops !=&ddsi_serdata_ops_builtin){ DDS_ERROR("Not associated with a built-in topic.\n"); ret = DDS_ERRNO(DDS_RETCODE_PRECONDITION_NOT_MET); } else { struct ddsi_serdata_builtin *d = tk->m_sample; nn_guid_t tmp=d->key; memcpy(guid,&tmp, sizeof(tmp)); ret=DDS_RETCODE_OK; } ddsi_tkmap_instance_unref (tk); } if (asleep) { thread_state_asleep(thr); } return ret; }I defined a declaration in
dds.hwith something like:
_Pre_satisfies_(entity & DDS_ENTITY_KIND_MASK) DDS_EXPORT dds_return_t dds_get_entity_guid ( _Out_ dds_builtintopic_guid_t *guid, _In_ dds_instance_handle_t *ihdl);
and tried to test this method in ping/pong. However I got nothing. I think thestruct ddsi_serdata_builtinand 'tk->m_sample' are not the same. Do you mean thenn_guid_t keyofddsi_serdata_builtinis the expected GUID？Where can I get the rightddsi_serdata_builtin?Happy New Year to you too :)
There are a couple of details, including a mistake I made in generating instance handles for participants — I guess I gave to much attention to the instance handles of endpoints ... oops!
- That mistake is the main problem. The entity creation code avoids generating entries in the key-value to instance-id map (
tkmap) for entity kinds defined as "built-in" kinds in the DDSI spec. I did that because I did not want to pollute the built-in topics ortkmapwith the various built-in endpoints for discovery, but I overlooked that the participant id is also classified as a "built-in". Oops. The result is that a different iid got allocated for the DCPSParticipant instance and so when you do a look up, there is nothing to be found. (Fixed it just now, so be sure to pull the latest change in.) [1] - The second issue is that the instance handle (a.k.a. instance id, a.k.a. iid) is just a 64-bit integer, so you have a type error passing
ihdl(a pointer to an instance handle) toddsi_tkmap_find_by_id. Chances that it will be found are vanishingly small (but not exactly 0). - Then there is a byte-order problem on little-endian machines. The
nn_hton_guidfunction converts from GUIDs-as-uint32_t-arrays to GUIDs-as-byte-arrays, but that involves a byteswap on little-endian machines. The byteswap is really only needed for the entity id interpretation, as everything else is completely meaningless, but to be consistent it swaps all 4 32-bit words. Without that conversion, you'll get a messed-up GUID.
With those changes, it works for me. Next time, I should perhaps do that simple check before I put some code in a comment 😁. That said, I am wondering if
dds_get_entity_guidis the best name after all: the other "get" operations on entities operate strictly on local entities (likedds_get_instance_handle), and this is based on full discovery information, including the local entities. Perhaps reflecting that distinction would be a good idea ... "lookup" instead of "get" might do that trick?However I got nothing. I think the struct ddsi_serdata_builtin and 'tk->m_sample' are not the same
Actually, they do match nicely. Each
tk->m_sampleis just addsi_serdatawhich can be of any "topic class", for want of a better word, but if you know which exact one you are looking for, you can check thetk->m_sample->opspointer (this being the equivalent of a vtable). "Normal" topics use a different set of operations, typicallyddsi_serdata_ops_cdr. So that test is sufficient.Finally, in my mailbox I had an earlier version of your comment, with the entirely reasonable question:
Is it because I forget to judge vtime_awake_p(thr->vtime)? Should I do this and why ?
I see you've fixed it, and exactly as I would have suggested. But that doesn't necessarily mean the "why" part is properly answered. It is quite possible that you have figured it out, but I'll answer the question anyway.
The background is that the DDSI core tries to satisfy three of my desiderata: (1) minimal reference counting (I hate having reference counts all over the place, in particular because of the overhead and risk of manipulating them all the time, but of course there are times when it is the best solution); (2) no "big kernel lock" serialising everything; and (3) being able to delete entities safely.
So instead of reference counting (or at least, manipulating reference counts on entities for each read/write/take operation), there is a "rule" that says you may only obtain a pointer to an entity by doing a lookup of its GUID while awake, and that pointer is then only valid for as long as you stay awake. After that, it may be garbage collected. [2] (The garbage collector in
q_gc.cis invoked explicitly, all it does is defer things until all threads are, or have been, asleep since queueing an entry for collection. A bit like the RCU mechanism in the Linux kernel, actually.) The result is that the vast majority of code can use pointers to entities without any complications.The tkmap reuses the mechanism. It too uses a concurrent hash table with lock-free lookups, and so has a big problem when it needs to resize or delete entries (this the sort of thing that's easy when you have a "real" garbage collector). Resizing means allocating a new, larger array of buckets, but freeing the old one must be deferred until no thread can still access the old one; deleting an entry means removing it from the hash table (covered by the concurrent hash table), and then ... what? Memory must be freed, but that again must be deferred. So the same rule applies to
tkmap_instance.The
vtime_awake_p(thr->vtime)assertion is therefore simply a partial check that this rule is adhered to. The reason for theif (asleep)by the way is that the listeners currently get invoked while awake, so it is not safe to unconditionally toggle thread's state. Certainly for some of the listeners this can be solved by change the precise time at which they're invoked, and so I do intend to have a good look at that and see if I can defer all listener invocations until the thread's no longer "awake" in this sense. Then, all these tests can go. If, on the other hand, it proves infeasible to make that change, I'll probable change the functions to change the state so that it has a counter. Then those tests can disappear everywhere else.As a side benefit, this toggling of awake/asleep states means one can have another thread that is pretty much completely independent of all the others and monitors whether the other threads do not stay awake for too long — so an internal detection mechanism for deadlocks. (See
q_servicelease.cfor that.)[1] Note that iids are unique in the system, you basically never run out of 64-bit integers. So it is 100% certain you will not be able to find the participant's instance id without my fix ...
[2] There are ways around it, look at deleting a proxy writer if you want to get the gory details.- That mistake is the main problem. The entity creation code avoids generating entries in the key-value to instance-id map (
Yes,
dds_lookup_entity_guidis much better and I have already changed it. Inpong.c, I usedinfo[j].instance_handleas the_In_ dds_instance_handle_t ihdlof this method. However, I always getNot associated with a built-in topic. I have no ideas about how to get remotedds_instance_handle_t. I have been thinking about the right way.Hi,
The
info[j].instance_handleof the RoundTrip topic is the unqiue identifier for the key value of the data;info[j].publication_handleshould be correspond to the instance handle of theDCPSPublicationtopic (as that identifies the writer). So I think you're just looking at the wrong entry in the sample info. Does that make sense?Just to be sure I hacked my tree a little bit to check this. I modified
pubsubto lookup and print the GUID from thepublication_handleinprint_sampleinfo, and the GUID it print matches what I see when I then also runddls -a, and so it appears that things line up nicely.{ dds_builtintopic_guid_t guid; dds_lookup_entity_guid (&guid, si->publication_handle); printf (" [%x:%x:%x:%x:%x:%x:%x:%x:%x:%x:%x:%x:%x:%x:%x:%x] ", guid.v[0], guid.v[1], guid.v[2], guid.v[3], guid.v[4], guid.v[5], guid.v[6], guid.v[7], guid.v[8], guid.v[9], guid.v[10], guid.v[11], guid.v[12], guid.v[13], guid.v[14], guid.v[15]); }Thanks for your help! We have finish a primary ignore strategy of ignoring a participant. We just preserve a GUID list which consists of GUIDs derived from the
ddsi_tkmap_find_by_idbypublication_handle. Then we check the GUID list at the top of the methodhandle_submsg_sequenceandreturn 0if we found it the right one to be ignored. We found it work right for different participants from difference processes. However, if we create one writer and one reader from the same participant and doignoreafter the reader reads (dds_read) info and get thepublication_handle, the ignore method is out of operation and the reader can still read info from the writer. How can I handle this situation. I need you help. Thanks!Hi @qqqshengwei, that's very good news!
The reason it doesn't work for writers and readers in the same process is that those take a shortcut via
deliver_locally(currently in dds_write.c). Having such a shortcut has the obvious advantage of having less overhead, but there are downsides too. You ran into one of them, the other one is in handling all kinds of edge-cases. So I'm not too certain this will remain exactly like this "forever".If there is only one participant in the process, you could simply skip that function. If there is only one participant in the process — which seems to be the case for most — it will be almost the same.
Now, Cyclone does allow you to create multiple participants in a process, and so the best implementation of
ignore_participantis to unmatch any readers and writers from the ignored one and prevent matching new ones. That is also relevant in your current implementation, because it also means there will be less useless state to keep around and that, e.g., lists of matching publications and subscriptions will be correct.- added 7 commits that reference this issue
on Apr 15, 2019 - added a commit that references this issue
on May 23, 2019 @qqqshengwei : I seem to assume that merging a commit referencing an issue would notify those who are watching the issue ... but I am not sure it actually works like that. So: Cyclone eventually gained a QoS setting on the reader/writer to ignore readers/writers in the same participant. All you now need to do is use
dds_qset_ignorelocal:- DDS_IGNORELOCAL_NONE: normal behaviour;
- DDS_IGNORELOCAL_PARTICIPANT: ignore readers/writers in the same participant;
- DDS_IGNORELOCAL_PROCESS: ignore readers/writers in the same process (which obviously only makes a difference if you have multiple participants in the same process).
If you're aware of it, great; if you're not yet aware of it, then it was about time I notified you of it.
Given this and the fact that there's been no further comment on this issue for three months, I'll close it.
Hi. I want to ignore a participant. I wonder where I can create an ignoreList and update it during the interaction between pubs and subs? By reading DDS Spec. (2.2.2.2.1.14). I found it as an irreversible operation. I hope that you can give me some implementation ideas.