Comments (10)
We running clang on CentOS9. The proxygen version v2024.04.01.00
should be new enough. Maybe try a TSAN build since this most likely a concurrency issue.
from presto.
@Yuhta FYI.
from presto.
We have not seen this error on 2000+ nodes for the last week. It must be specific to some network configurations or proxygen version.
from presto.
I'm able to see it on a local dev environment but once compiled with debug symbol it is not showing up anymore. This is on CentOS9 Linux with gcc12. I've tried to repro on MacOS with clang but wasn't able to so far. What are you building with?
The proxygen version used is the one from the current setup scripts in prestissimo v2024.04.01.00
. So should be the same that you are using or are you using a newer one? We want to update the FBOS dependencies in Velox (and as a result proxygen) to the May 20th version.
from presto.
I was able to reproduce this and can confirm that auto* evb = session->getEventBase();
is causing the segfault. Removing that line does not cause the sigsegv.
I removed this line and added VELOX_CHECK(session->getTransport() != nullptr)
to check for the value of sock_
and this would fail as well.
Looking at the HttpSessionBase API, should we not check if the session is ready for reuse?
I added this check and it fixes the problem. #23018
However, I tried to check if there was any reuse after this change, and I did not see the reuse log message.
@Yuhta any thoughts on this?
if (!session || !session->isReusable()) {
return nullptr;
}
from presto.
A simpler fix is to perform the checks if the session is reusable. The fact that the session is not yet reusable makes me wonder if there is a bigger problem here.
if (session->isReusable()) {
auto* evb = session->getEventBase();
// The event base from idle session should not be the current event base,
// otherwise we should have already got it from the local session pool.
VELOX_CHECK(!evb || !evb->isInEventBaseThread());
}
from presto.
@Yuhta can you share your thoughts here? Is it safe to add a session to the sessionPool_
when the session is not yet reusable?
from presto.
@majetideepak It seems isReusable
is always false in this case, so we cannot do that. Can you file a bug report with the repro with proxygen (https://github.com/facebook/proxygen/issues)? HttpSession
we got from ServerIdleSessionController
should be guaranteed to be reusable.
In the meantime you can disable connection pool (exchange.http-client.enable-connection-pool
) for your instances.
from presto.
Closed by mistake
from presto.
Meta doesn't see problems using this code but the compiler used is clang (an older version).
The statements causing the segv are validation statements and when removed don't cause issues. Subsequent runs show no further issue.
The TSAN did not show a race condition in this area even after the fix.
Next step: Use clang 17 and see if the generated code causes reproduces the issue as it does with gcc12.
from presto.
Related Issues (20)
- Add ARM64 Support for Building Prestissimo Docker Image on Mac M1 HOT 7
- Backport https://github.com/prestodb/presto/pull/22926 into 0.285, 0.286 and 0.287 HOT 4
- Pushdown (partial) rowNumber under join
- Flaky test: TestMemoryManager.testReservedPoolDisabledMultiCoordinator
- Add documentation for Geospatial types in main types page HOT 1
- For each agg function with input param as <T>, Add an equivalent agg function with input param as array<T> HOT 1
- [docs] Combine the descriptions of session property with configuration property for history based optimization
- singlestore-dockerized-tests job is failing often HOT 1
- Getting error while building in intelli idea HOT 1
- How to build a custom connector?
- How to build and run presto in intellij idea? HOT 1
- [native] Flaky test TaskManagerTest.buildSpillDirectoryFailure HOT 2
- Writer scaling fails for Parquet with smaller files HOT 5
- Flaky test: TestNoisySumGaussianLongAggregation.testNoisySumGaussianLongClippingSomeNoiseScaleWithinSomeStd() HOT 1
- Iceberg $changelog read fails on table with only one snapshot version.
- Pushdown partial TopN and RowNumber into UNION
- Inline cosntant cross joins
- Allow Presto Coordinator to ignore (not throw) negative runtime metrics. HOT 1
- Update the MongoDB connector to support binData data type HOT 2
- Presto needs a modern functional testing framework that runs tests using real infrastructure
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from presto.