From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pete Zaitcev Subject: Re: [Patch 5/7] tabled: Add replication daemon Date: Sat, 14 Nov 2009 21:53:18 -0700 Message-ID: <20091114215318.7b1c4eea@redhat.com> References: <20091113233605.673bfb9e@redhat.com> <4AFF40DA.5020505@garzik.org> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <4AFF40DA.5020505@garzik.org> Sender: hail-devel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii" To: Jeff Garzik Cc: Project Hail List On Sat, 14 Nov 2009 18:44:26 -0500, Jeff Garzik wrote: > 1) What is the point of db->del() in rep_add_nid() ? You are in the > middle of a transaction, and you immediately overwrite that record in > the same transaction. That is clearly unnecessary work, when db->put() > will simply overwrite an existing record, if requested. That didn't work last I tested, but I forgot what the issue was. The code is copied from object.c. I'll review it. > 2) rep_scan(): I would rather not make the entire daemon non-responsive > for multiple seconds. > > The database is already in multi-threaded mode, and db4 is > free-threaded, so it would seem to make a lot more sense to simply > g_thread_create() a thread to do this work. True, but multi-threading in tabled is a much bigger undertaking than just "simply" creating a thread. There's a lot of common state. I think the 2s problem is less urgent than processing massive replication more efficiently (from a Chunk down). > Also, there should be no need to scan a chunkd's keys at all, as long as > the chunkd instance is still communicating with us. No, there's a need, although there's no code for it: the scan should probe keys by requesting their metadata, which forturously includes checksums. This is how the results of Chunk's self-test will be communicated to tabled. -- Pete