ttttttz has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21418 )

Change subject: IMPALA-12705: Add /catalog-ha-info page on Statestore to show 
catalog HA information
......................................................................


Patch Set 9:

(7 comments)

Hi, Wenzhe! Thank you for your guidance! I have processed all review comments 
and committed the latest code.

http://gerrit.cloudera.org:8080/#/c/21418/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/21418/6//COMMIT_MSG@7
PS6, Line 7: Statestor
> nit: typo statestore
Ack


http://gerrit.cloudera.org:8080/#/c/21418/6//COMMIT_MSG@9
PS6, Line 9: Statesto
> typo statestore
Ack


http://gerrit.cloudera.org:8080/#/c/21418/6//COMMIT_MSG@11
PS6, Line 11:
> nit: wrap up long line
Ack


http://gerrit.cloudera.org:8080/#/c/21418/6//COMMIT_MSG@20
PS6, Line 20:
> Should we add unit-test for this new page?
Ack


http://gerrit.cloudera.org:8080/#/c/21418/6/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:

http://gerrit.cloudera.org:8080/#/c/21418/6/be/src/statestore/statestore.cc@652
PS6, Line 652:   catalogd_version_ = catalogd_version;
> need to acquire lock before updating.
The subscribers_lock_ has been acquired before entering the 
Statestore::Subscriber::UpdateCatalogInfo() method.


http://gerrit.cloudera.org:8080/#/c/21418/6/be/src/statestore/statestore.cc@2245
PS6, Line 2245:   standby_catalogd_registration_time, document->GetAllocator());
              :     }
> can replace with subscriber.second->IsSubscribedCatalogdChange()
Ack


http://gerrit.cloudera.org:8080/#/c/21418/6/common/thrift/StatestoreService.thrift
File common/thrift/StatestoreService.thrift:

http://gerrit.cloudera.org:8080/#/c/21418/6/common/thrift/StatestoreService.thrift@222
PS6, Line 222: registration_time
> This is timestamp, not time period. Name with appendix '_ms' sounds like pe
Ack



--
To view, visit http://gerrit.cloudera.org:8080/21418
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If85f6a827ae8180d13caac588b92af0511ac35e3
Gerrit-Change-Number: 21418
Gerrit-PatchSet: 9
Gerrit-Owner: ttttttz <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Reviewer: ttttttz <[email protected]>
Gerrit-Comment-Date: Mon, 27 May 2024 03:10:09 +0000
Gerrit-HasComments: Yes

Reply via email to