Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries ...................................................................... Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.h File be/src/runtime/coordinator.h: http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.h@301 PS6, Line 301: /// Synchronizes updates to the filter_routing_table_. : /// Any thread which attempts to update the filter_routing_table_ must acquire : /// this lock after acquiring the filter_lock_ in shared mode. However, if a thread : /// has exclusive access to filter_lock_, it need not acquire the : /// filter_update_lock_. : SpinLock filter_update_lock_; : : /// Protects filter_routing_table_. : /// Any thread which attempts to access the filter_routing_table must acquire the lock : /// in shared mode. Additionally, any thread which alters the filter_routing_table_ : /// must acquire the lock in exclusive mode. maybe write a consolidated comment to better explain the need for two locks. This would also help to avoid writing explanations in the .cc file every time we acquire these locks: Protects filter_routing_table_. Usage Pattern: 1. To update filter_routing_table_: Acquire shared access on filter_lock_ and upgrade to exclusive access by subsequently acquiring filter_update_lock_. 2. To read, initialize/destroy filter_routing_table_: Directly acquire exclusive access on filter_lock_ http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.h@334 PS6, Line 334: /// This function must only be invoked by threads which have acquired : /// exclusive access to the filter_lock_. If the caller has not acquired : /// the filter_lock_. the results produced would be undefined. nit: "Caller must have exclusive access to filter_lock_" http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.cc@265 PS6, Line 265: // Since this function alters the filter_routing_table_ by allocating memory for the : // filter objects, acquire the filter_lock_ in exclusive mode to ensure that no other : // thread accesses the memory before it is initialized. fyi: this method is called in exec before any fragments are started. Also, no other coord methods are called before exec returns. -- To view, visit http://gerrit.cloudera.org:8080/11005 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c Gerrit-Change-Number: 11005 Gerrit-PatchSet: 6 Gerrit-Owner: Pooja Nilangekar <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Pooja Nilangekar <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 03 Aug 2018 00:55:17 +0000 Gerrit-HasComments: Yes
