Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16687 )
Change subject: [client] KUDU-2966 configure connection negotiation timeout ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/16687/1/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/16687/1/src/kudu/client/client-test.cc@898 PS1, Line 898: auto s = b.Build(&c); : ASSERT_OK(s); > nit: inline the ASSERT? Same below Done http://gerrit.cloudera.org:8080/#/c/16687/1/src/kudu/client/client-test.cc@911 PS1, Line 911: timeout > nit: kConstNamingScheme? Done http://gerrit.cloudera.org:8080/#/c/16687/1/src/kudu/client/client-test.cc@919 PS1, Line 919: Initialized > nit: isn't this taken care of by the comparison? Yes, but those comparison operators have DCHECK(Initialized()) in their body, so I was thinking avoid a crash in DEBUG configuration if this ever regresses. From the other side, crash would certainly point to the issue as well :) http://gerrit.cloudera.org:8080/#/c/16687/1/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/16687/1/src/kudu/client/client.h@268 PS1, Line 268: If not provided, the underlying messenger is created with reasonable : /// default. The result value could be retrieved using : /// @c KuduClient.connection_negotiation_timeout() after an instance of : /// @c KuduClient is created. > nit: Would it make sense to add a small blurb about what connection negotia Done -- To view, visit http://gerrit.cloudera.org:8080/16687 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8573a572887be041637e28e518a7cd3a6d1f1693 Gerrit-Change-Number: 16687 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 04 Nov 2020 02:24:10 +0000 Gerrit-HasComments: Yes
