MichaelJCaruso
commented
5 years ago We may have seen something like this before.
Open VCommitter opened 5 years ago
MichaelJCaruso
commented
5 years ago We may have seen something like this before.
VCommitter
commented
5 years ago Similar for sure. Though in this case I don't think poll() is lying to us:
POLLOUT
Writing is now possible, though a write larger that the avail‐
able space in a socket or pipe will still block (unless O_NON‐
BLOCK is set).It has written 1.2 megs of 1.9. Presumably it's peer that is also blocked on a large write needs to do some reading to drain the TCP/IP buffers before the last 0.7 megs can be written.
Instead of a timeout based solution we could set the device to non-blocking. Right now setNonBlockingTo() is implemented at the Vca::OS::Device level which should work for a socket file descriptor. Non-blocking is supported by all file descriptors where write timeouts are only supported by sockets via setsockopt().
bool Vca::OS::Device::setNonBlockingTo (bool bNonBlocking) const {
int iFlagsNow = fcntl (m_hDevice, F_GETFL);
int iFlagsNew = bNonBlocking ? iFlagsNow | O_NONBLOCK : iFlagsNow & ~O_NONBLOCK;
return iFlagsNow != -1 && (iFlagsNow == iFlagsNew || fcntl (m_hDevice, F_SETFL, iFlagsNew) != -1);
}We may have seen something like this before.
I have a question about the code near this comment. If the fix is to make the device non-blocking it looks like a fix was made and then commented it out? Or was it implemented some other way and this experimental non-blocking attempted fix was discarded?
MichaelJCaruso
commented
5 years ago Good question. Wish the breadcrumbs weren't so stale :-). It's possible that the fix was sound, but difficult to test properly and so wasn't deployed. It's a long shot, but do the old Perforce logs still exist?
dialtr
commented
5 years ago @MichaelJCaruso Are you referring to the Perforce history? I believe we should still have that.
MichaelJCaruso
commented
5 years ago @dialtr Yes. I don't know how accessible and searchable that history is, but if the change I referenced in my before link a couple of comments back can be found, there's a small chance that the commit comment may say something more useful than checking in a bunch of changes :-). If we're lucky, it may even say something like preserving code for a proposed change that we don't have time to test now but should release once we do. It's a long shot and probably not worth spending too much time on except for the historical audit value.
jarena
commented
5 years ago @MichaelJCaruso the makeDeviceFriendly() method was introduced back on 05-FEB-2004 with the change description:
Promote:
- O_NONBLOCK Unix/Linux devices.
- Minor routine renaming.
- Further vca.xfer pruning.
At that time, it included the call to makeNonBlocking().
A few weeks later on 01-MAR-2004, the call to makeNonBlocking() was commented out, but the change description was not helpful:
Move VInterfaceMember into Vca namespace, rename VInterfaceMember... to Vca_VInterfaceMember...
6+ years later, on 19-JUL-2010, the call to makeNonBlocking() was re-introduced with the change description:
Make serial Vca devices NON-BLOCKING on Unix/Linux. Historically our practice has been to leave these devices in their default blocking state and use 'poll' to determine when calls that would otherwise block can be expected to return immediately. Unfortunately, in recent Linux testing, 'poll' has failed us. The problem is easily reproduced, occuring in processes that have served as middle-men in multi-hop communication (e.g., a pool) when those processes are closing their connections prior to exiting.
This was the change that added in the large comment block that you referenced earlier.
And just 4 days later, on 23-JUL-2010, the call to makeNonBlocking() was commented out again with the change description:
Revert the decision to make devices non-blocking on Unix/Linux. I've seen more problems with non-blocking devices (at least with terminal devices) than I have with what was there and have come not to trust it as a result.
VCommitter
commented
5 years ago I pushed a branch for the timeout fix and a branch for the nonblocking fix. These are both passing preliminary tests in our problem scenario.
Unfortunately I can't share my reproducer because it's proprietary. @MichaelJCaruso perhaps you can try either of these fixes out against some of the hangs you've been seeing?
MichaelJCaruso
commented
5 years ago Thanks @jarena!!! Commit commentary shortcomings aside, that history is exactly what is needed.
VCommitter
commented
5 years ago Given the discussion here and in other threads I propose the following fix:
Non-Blocking fixes the problems I've been encountering as well as a timeout on the socket; without imposing the performance cost of 30 second timeouts.
It's also simpler to get thorough testing of nonblocking: every operation that succeeds over a socket is testing nonblocking reads and writes. Timeouts on the other hand present testing complications as different timeout sizes and I/O operations sizes will combine to many different scenarios that need coverage.
I will proceed with this fix.
VCommitter
commented
5 years ago Here's another comment @MichaelJCaruso found in Vca_VDeviceFactory.cpp that also seems related:
/********************************************************************************
* Note: Blocking socket descriptors created using Linux's socketpair ()
* has problems (hangs) when writing large chunks of date. To avoid it
* we have written this local getSocketPair () which is called with AF_INET
* family, to create connected pair of INET domain sockets
********************************************************************************/
I've been having some trouble with Vca communication locking up between two peers when large writes do not complete. Basically both peers wind up in the middle of the
writecall inVca::OS::DevicePoll::doWrite. It seems like one is waiting for the other. The backtrace on both ends looks something like this:It is not limited to writes on
Vca::VBSProducerConsumerDevice_<Vca::OS::SocketDevice>- it can also lock up on writes toVca::VBSConsumerDevice_<Vca::OS::Device>:Attaching a debugger to either peer in this process interrupts the locked
write(). Oncecontinueed the peers unlock and complete their operations with no lingering problems.Inspecting a locked
Vca::OS::DevicePoll::doWriteshows you that thewrite()has already written some data but seems to get stuck before completing the requested write. Here is an example where 1,172,880 bytes (rsTransfer) have been written of a call towrite()of 1,995,938 bytes (sRequest):Proposed Fix
I propose that we add a timeout to calls to
write()in Vca - specifically those calls towrite()inVca::OS::DevicePoll::doWrite. This will keep peers from blocking indefinitely on large writes. Once the hungwrite()is timed out Vca's I/O scheduling system is able to successfully complete the I/Os scheduled between peers.This
write()timeout will serve to make Vca's communication more robust. The timeout may resolve other hangs that I haven't reproduced in my own testing.