Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
I decided to leave setDetached and not deprecate it. I don't see a reason
for someone to change from detachable to joinable (or the reverse) while
running but I believe all of the servers will
Github user ben-craig commented on the issue:
https://github.com/apache/thrift/pull/1103
I'm fine with you adding back setDetached and marking it deprecated. If
you really want, you could even put in some logic so that you assert or throw
an error if someone calls setDetached after
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
What you described is a failing in the original set of thread factories to
provide common functionality (detached, policy, priority) in the base class.
Only PosixThreadFactory has policy and
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
@ben-craig changing the "detached" behavior of an active thread manager
seems like a bug to me which is why I removed it. What is the use case for it?
---
If your project is set up for it, you
Github user ben-craig commented on the issue:
https://github.com/apache/thrift/pull/1103
Removing setDetached and the old thread factory ctors breaks client code
loudly. It doesn't fix the underlying problem, and it exchanges one problem
(don't forget to set the detached mode!) for
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
How will a decision be made as to whether this gets into 0.10.0 or not?
Obviously it is not a trivial change but I think it solidifies parts of the C++
engine that have been neglected; adds much
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
I re-enabled these tests and they have all passed in Travis and in
Appveyor. I also brought the time on TInterruptTest down by 90 seconds. The
entire suite runs in about 60 seconds on Windows
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
I made this single change:
> +++ b/lib/cpp/test/processor/ProcessorTest.cpp
> uint32_t checkNewConnEvents(const boost::shared_ptr& log) {
>// Check for an ET_CONN_CREATED event
Github user nsuke commented on the issue:
https://github.com/apache/thrift/pull/1103
Oh, good catch, concurrency_test definitely needs to be enabled for Linux
cmake CI and autotools.
Somehow thought that you were referring to concurrency_test in the last
comment,
but
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
https://travis-ci.org/apache/thrift/builds/164436835
Job 3025.17
https://travis-ci.org/apache/thrift/jobs/164436856 has in it:
ctest -VV -E "(concurrency_test|processor_test)"
Are we
Github user nsuke commented on the issue:
https://github.com/apache/thrift/pull/1103
It looks succeeding to me :tada:
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
It looks like the processor_test is failing now. ;(
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
I reverted the PosixThreadManager enum changes per @nsuke 's request.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
I pushed a fix for the failing monitor timing test as described above; this
resolves the appveyor issue with concurrency_test that was previously
discovered. I intend to leave concurrency_test
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
The latest AppVeyor build failed because the thread factory monitor test
assumes that a monitor wait(milliseconds) will wake up at "milliseconds"
however the only guarantee is that it will sleep
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
Okay disregard that comment about boost... it turns out the blockTest was
using a std::set to store the tasks it put into the thread manager so they were
getting reordered. I changed it to a
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
By removing the THRIFT_SLEEP_USEC(1) calls in the test it gets much faster,
which is great, however as part of this effort I have discovered our boost
implementation is broken in some way. I
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
I think I found the reason for the poor test performance on Windows - the
minimum granularity of a sleep call on Windows as defined by thrift_usleep() is
1 millisecond. So in a test where we
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
It looks like we have a number of tests disabled on Appveyor that were not
behaving well... I'm going to re-enable as many as I can since they seem to be
pretty stable for me locally, starting with
Github user jeking3 commented on the issue:
https://github.com/apache/thrift/pull/1103
It is pretty frustrating when you build, test, repeat many many times and
everything looks good, and then the CI build fails, but that's why it's there
right?
---
If your project is set up for
20 matches
Mail list logo
