I've looked at core/modules/replica/AddReplicaQservMgtRequest.cc through core/modules/replica/Controller.h And I'm calling it quits on this for the day.

Hi Igor Gaponenko, I'm not a C++ expert but thanks for adding me to this review where I learned a log about this replication system. I've added a few minor comments and removed myself from reviewers list. We should think on how to integrate these new services inside Kubernetes in the long term... Your health probe algorithms might also be triggered inside Kubernetes probes...

Regards,

Fabrice

I think I have done my part reviewing the ticket (or at least I have to stop somewhere to return to my work), there are plenty of comments on PR.

I really do not know what to say. I trust replication part should be OK as it is was tested at PDAC. What worries me is the database access code. As it happens we now have two separate APIs for mysql interface in qserv and they both are very far from perfect. I do not think that we can afford to support and improve both so we should instead have one reasonable interface with good quality implementation. I really wish that Igor worked on improving our existing sql+mysql implementation rather than rolling out completely new code. Anyways I think that for long term we should converge on a single implementation, either one of our own or maybe some other third-party package (and for completeness I should also mention option I like best - rewriting replicator in Python  )

Removing myself from reviewers. 

Andy Salnikov thank you very much for putting so much effort into reviewing the ticket! Most of your comments and suggestion on my code (as always) make a lot of sense to me.
I would also like to address your last comment on the database APIs. Even though, I would agree (in general) with your argument that we may need a better API, I should also mention the following:

1. a choice of the database technology for supporting a persistent state of the Replication system is not that critical as in case of Qserv. The Replication system's state can be well contained within virtually any relational (and not only) database. I'm pretty sure multiple strains of MySQL will be around for many years ahead. Moreover, the Docker technology makes this long-term investment (into MySQL) even more reasonable.
2. I don't see much benefits for building a database abstraction layer on top of MySQL within the Replication system. That's why I've put my efforts in building a MySQL-specifc OO-layer on top of the low-level C library (for MySQL) based primarily on my needs and requirements. Also note that my implementation is quite limited. And it's not aiming at being a general purpose solution to all problems.
3. at the mean time, requirements to the database abstraction layer in Qserv are quite different. Having a technology-neutral API could be one of those. Besides, this API should also allow efficient implementations for reading large amounts of data from a database.
4. I also believe that building a high quality general purpose database API (including a technology-neutral abstraction layer and efficient underlying implementation) is a very big job. It may require substantial efforts to design and implement it. Based on my informal cost-benefit analysis, I don't think it's worth it. Or, we may not just afford this development.
5. And finally, we've already discussed a possibility of decoupling the Replication system from Qserv code base and placing it into a separate Git package. This will make code sharing even less possible from a practical standpoint (actually, as of today, there is very little overlap between Qserv and the Replication system)

between Andy Salnikov and I all the files have been looked at and comments have been left where appropriate. We will let Igor Gaponenko make changes and submit as he sees fit.