Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/22215 )
Change subject: IMPALA-13478: Sync tuple cache files to disk asynchronously ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/22215/4/be/src/runtime/tuple-cache-mgr-test.cc File be/src/runtime/tuple-cache-mgr-test.cc: http://gerrit.cloudera.org:8080/#/c/22215/4/be/src/runtime/tuple-cache-mgr-test.cc@308 PS4, Line 308: TestSyncToDisk Can we have a testcase for tuple_cache_sync_pool_queue_depth is exceeded? http://gerrit.cloudera.org:8080/#/c/22215/4/be/src/runtime/tuple-cache-mgr-test.cc@318 PS4, Line 318: SleepForMs(100); Would it be possible to be flaky here? http://gerrit.cloudera.org:8080/#/c/22215/4/be/src/runtime/tuple-cache-mgr.cc File be/src/runtime/tuple-cache-mgr.cc: http://gerrit.cloudera.org:8080/#/c/22215/4/be/src/runtime/tuple-cache-mgr.cc@517 PS4, Line 517: std:: nit. no need for std http://gerrit.cloudera.org:8080/#/c/22215/4/be/src/runtime/tuple-cache-mgr.cc@545 PS4, Line 545: // Only crash for a failed state change on debug builds. The sync completed : // and the state change doesn't really impact external behavior. It isn't : // worth crashing on a release build. Would it be better if we log a warning for release build? -- To view, visit http://gerrit.cloudera.org:8080/22215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I646bb56300656d8b8ac613cb8fe2f85180b386d3 Gerrit-Change-Number: 22215 Gerrit-PatchSet: 4 Gerrit-Owner: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Yida Wu <wydbaggio...@gmail.com> Gerrit-Comment-Date: Wed, 09 Apr 2025 18:19:50 +0000 Gerrit-HasComments: Yes