BewareMyPower opened a new pull request #11762:
URL: https://github.com/apache/pulsar/pull/11762
Fixes #11760
### Motivation
`BasicEndToEndTest.testLookupThrottling` is flaky, from the logs of #11760
we can see the test crashed in `ClientImpl::shutdown`. The `Connection closed`
log shows that `ConnectionPool` has been closed, so the problem is related to
the close of three `ExecutorServiceProvider`s of `ClientImpl`.
`ExecutorServiceProvider::close` is not thread safe because
`ExecutorServiceProvider::get` might access the `executors_` field in different
threads and `get` uses a lock to guarantee thread-safety while `close` doesn't.
In addition, some clean up methods including `ClientImpl#shutdown` and
`ExecutorService#close` might be called twice or more, which is not necessary
and has potential bugs.
### Modifications
The main changes are:
- Add a lock to guarantee thread-safety of `ExecutorServiceProvider::close`.
- Use an atomic variable to make sure the logic in `ExecutorService#close`
is called only once.
- Don't call `shutdown()` method in `ClientImpl`'s destructor to avoid
`shutdown()` being called twice.
In addition, this PR involves other changes. First, the master branch's
source code cannot be compiled in my local env (macOS + Clang 12.0.0 + Boost
1.74), the error is:
```
/usr/local/include/boost/proto/expr.hpp:140:44: error: unknown warning group
'-Wdeprecated-copy', ignored [-Werror,-Wunknown-warning-option]
#pragma GCC diagnostic ignored "-Wdeprecated-copy"
```
I followed https://github.com/boostorg/proto/issues/30 to fix this
compilation error.
Another change is to fix the logs format, we can see the filename is always
the same from logs in #11760. The reason is #11668 has changed `logger()`
method from `static` to `inline`. It will make all C++'s translation units
share the same and random chosen definition of `logger()`, but we need
different definitions with different `__FILE__` in different translation units.
So this PR modified `static` back to `inline` and fix related compilation
errors.
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
This change is a trivial rework / code cleanup without any test coverage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
