Alexey Serbin has posted comments on this change.
Change subject: client/sample.cc: fixed a couple of crashes
..
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/3685/1//COMMIT_MSG
Commit Message:
Line 11: while logging some messages from terminating reactor threads.
> is there another way we can fix this without forcing callers to uninstall t
I could think of making the logger callback static with lifespan longer than
those objects in the main() scope. However, I'm not sure that will guarantee
the desired behavior: I'm not sure there is a guarantee the reactor threads
will be dead at time destruction of that static object.
Another thing I'm thinking is to create a wrapper object which would install
the logger callback in constructor and un-install the callback in its
destructor (no exceptions in case of failures, though -- only messages to
std::cerr).
But those are about working around the real issue of shutting down reactor
threads independently in the background. Probably, the destructor of
KuduClient should await for completion of those reactor threads instread.
Line 11: while logging some messages from terminating reactor threads.
> As with the below, any ideas as to why the precommit test didn't experience
There is a catch: I modified the sample.cc to run with different parameters.
That exposed this bug. I built the source in debug configuration. If needed,
I can send you the patch.
Basically, I played around the sample.cc building different scenarios to run
against the original client library code and code with my recent modifications.
The issues fixed in the patch appear even in client library with no
AUTO_BACKGROUND_FLUSH support.
Line 13: Fixed issue with an attempt to access non-existing element
> hmm, the bug fix looks reasonable, but I'm curious why we don't see this cr
Please see the response for the prior comment: in essence, I run modified
sample.cc and that exposed the bug.
Line 13: Fixed issue with an attempt to access non-existing element
> I'm also surprised. front() on an empty vector is undefined behavior, so ma
I found that going through different scenario in sample.cc, basically modified
the sample.cc code.
--
To view, visit http://gerrit.cloudera.org:8080/3685
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic5fa3b812e6402a113bf5e432a3a451dc4cc3735
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes