Sailesh Mukil has posted comments on this change. Change subject: IMPALA-4456: Change query_exec_state_lock_ to a reader-writer lock ......................................................................
Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/5021/2//COMMIT_MSG Commit Message: PS2, Line 11: wi > with Done PS2, Line 13: The main offender, ReportExecStatus() contends heavily for this lock : on the coordinator for each update that a fragment instance sends as : a RPC. This causes the creation of a larger number of new client : connections on that node as a lot of connec > Quantify what "big win" means. Done http://gerrit.cloudera.org:8080/#/c/5021/2/be/src/service/impala-server.h File be/src/service/impala-server.h: Line 673: /// being modified, in which case it must be acquired as a write lock. > Update comment with expected read and write usage, and also update class co Done http://gerrit.cloudera.org:8080/#/c/5021/2/be/src/util/rw_lock.h File be/src/util/rw_lock.h: PS2, Line 25: class ReadWriteLock { : public: : ReadWriteLock() {} : : // Reader lock functions. : void ReadAcquire() { l.ReadAcquire(); } : void ReadRelease() { l.ReadRelease(); } : : // Writer lock functions. : void WriteAcquire() { l.WriteAcquire(); } : void WriteRelease() { l.WriteRelease(); } : : private: : base::subtle::ReadWriteLock l; : : DISALLOW_COPY_AND_ASSIGN(ReadWriteLock); : }; > Why does this class need to exist? I noticed that we don't have the habit of using objects from other namespaces (save for boost or std). If you think it doesn't matter using the base::subtle namespace in the main codebase, then I can change it to using it directly from the gutils. -- To view, visit http://gerrit.cloudera.org:8080/5021 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I790a95e7179f07aa7ba188d5422c5e054353ba0b Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-HasComments: Yes
