[Cl] Do not apply timeout of Channel to its redirect-collapsed replacement #2549
Conversation
|
This PR is to change a current behavior in XrdCl concerning redirects that make use of the kXR_collapseRedir facility: The intent of the kXR_collapseRedir feature is that a hostname like 'xrootd-prod.dom' which might resolve to e.g. xrootd1.dom and xrootd2.dom, and will cause the client to connect to one of those. e.g. xrootd2.dom. That host could indicate in a redirect message that for the duration of the client's Channel for 'xrootd-prod.dom' it should refer to a conneciton to one of the other hosts, e.g. 'xrootd1.dom'. (This might be part of the operaiton of a HA setup). Currently the client will replace the channel for 'xrootd-prod.dom' with one definitly for 'xrootd1.dom' as expected, and the old channel will become idle, and eventually timeout with TTL, also as expected. However currently that timeout will cleanup the replacement channel (which may be in use), and infact since the idle channel is not actually destroyed the timeout will repeate for every subsequent timeout check interval. This PR as currently written arguably adds some techincal debt; this PR makes PostMaster::ForceDisconnect change bevahior depending on &url. A better alternative appears to be adding a Channel* argument to ForceDisconnect (but to avoid breaking ABI of the public 'PostMaster' class, that would probably need to be an additional ForceDisconnect() method, there are already two. And XrdStream may need a new member to contain XrdChannel*, so that it can be passed at the point it calls ForceDisconnect). Any suggesiton to improve that is welcome. Or the approach in this PR could be adopted for now, and improved at release 6. |
|
@smithdh Could you please add the description above to the commit message? Thank you! |
f875d06 to
c8f1024
Compare
c8f1024 to
698b338
Compare
|
hello, I added the commit message. |
…ement This commit is to change behavior in XrdCl concerning redirects that make use of the kXR_collapseRedir facility: The intent of the kXR_collapseRedir feature is that a hostname like 'xrootd-prod.dom' might resolve to multiple addresses, e.g. for xrootd1.dom and xrootd2.dom, and will cause the client to connect to one of those. e.g. xrootd2.dom. That host could indicate in a redirect message using collapseRedir that for the duration of the client's Channel for 'xrootd-prod.dom' the connection should refer to one of the other hosts, e.g. 'xrootd1.dom'. This might be part of the working of an HA setup. Currently the client will replace the channel for 'xrootd-prod.dom' with one for 'xrootd1.dom', as expected and the old channel will become idle and eventually timeout with TTL, also as expected. However currently that timeout will cleanup the replacement channel (which may be in use) and since the idle channel is not destroyed the timeout will repeat for every subsequent timeout check interval. This commit arguably adds some technical debt; it makes PostMaster::ForceDisconnect change behavior depending on &url. A better alternative appears to be adding a Channel* argument to ForceDisconnect (but to avoid breaking ABI of the public 'PostMaster' class, that would probably need to be an additional ForceDisconnect() method, there are already two. And XrdStream may need a new member to contain XrdChannel*, so that it can be passed at the point it calls ForceDisconnect). It may be worthwhile to revise this at a point where breaking ABI, e.g. for release 6.
698b338 to
bb40de4
Compare
|
ABI check was fine, so I'm merging this directly into |
No description provided.