otterc commented on pull request #30139: URL: https://github.com/apache/spark/pull/30139#issuecomment-717341340
> I know that, but as @jiangxb1987 @mridulm mentioned, we need to ensure the streamState and the totalChunksBeingTransfered are updated synchronically. Add this lock is a strong guarantee. The execution process in the middle of the lock is very fast, so the impact is not really significant. Also, It's a huge leap in performance compared to what it was before Speeding up `chunksBeingTransferred` while ensuring strong consistency with introducing this lock on an update of every streamstate has a cost. The cost here is that we are increasing the time it to takes to update the streamstate. In a production like environment with a lot of concurrent streams, this will increase the time of `chunkSent` and `chunkBeingSent` > The execution process in the middle of the lock is very fast, so the impact is not really significant. With this strong consistency enforced, have you scale tested it? I haven't run this change, but I am quite sure that the impact on `chunkSent` and `chunkBeingSent` will be significant. > Also, It's a huge leap in performance compared to what it was before The improvement in performance is on read of `chunksBeingTransferred`. But, as above earlier when you add strong consistency by introducing this lock it will degrade the performance of `chunkSent` and `chunkBeingSent`. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
