On Wed, Feb 27, 2019 at 6:01 PM Ben Pfaff <[email protected]> wrote: > > I spent a bunch of time looking through this series. The actual support > for fast resync is really simple and clean. > > The first patch is the one that I have the most trouble with. I think > it's probably because I don't understand the existing code all that > well. I wrote the original monitor implementation, then other people > added the caching layer, and I think I reviewed that caching layer > without ever properly understanding it. Do you understand it? Are you > confident that your revision is high quality, and are you confident that > you can maintain it? > > Thanks, > > Ben.
Hi Ben, Thanks a lot for spending time reviewing this. For the first patch: "ovsdb-monitor: Refactor ovsdb monitor implementation.", I am pretty confident on it. I see spaces for further improvement, but at least I am sure the revised version is functionally equivalent to the original one. The monitoring implementation was hard to understand for me, too. After spending a lot of time on it I understand the structure and most of the details now, and I believe it is slightly easier to understand after the refactoring, and I think I can maintain it. Consider my previous bug fix on the crash caused by condition change as an evidence :o) I think I can add some documentation on it (so that I will not forget after some time). And thanks for finding the Address Sanitizer test failure. I think I got the root cause, which is related to the order of freeing the rows tracked in the txn history and the table schema that is used. I will figure out how to properly fix it. Thanks, Han _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
