Code review comment for lp:~letterj/swift/defaultbacklog

Revision history for this message
clayg (clay-gerrard) wrote :

Of little consequence; there's some debate on proper placement of the backlog setting in the config. Does it belong in the app section? Or should it be in the default section next to the other socket related options - bind_ip and bind_port?

After some discussion, it seems that the "bind_port" is used by the replicator to determine which devices in the ring it's responsible for replicating. In many cases comparing the 'ip' of the device in the ring to the ips configured on the system running the replicator (see common.util.whataremyips) is enough for the replicator to "claim" a device - but not always (e.g. saio) - so the bind_port in the config is used as final verification key.

To keep things less weird looking it seems a few other app/server config options were moved into default section as well (i.e. bind_ip and workers - which are all used in run_wsgi around the same time as bind_port) - although I can't see where any of them besides bind_port are used in any other module classes.

IMHO, all the socket and run_wsgi related config options should probably be kept together in the default section of the config instead of the app/server section - but my only justification is keeping with the convention of "less weird looking".

If there's time, it may be nice to try and track down some other "config option easter eggs" that could added to the conf-sample's (e.g. object-replicator.ring_check_interval) and some defaults that don't match the configs (e.g. object-replicator.timeout)

review: Needs Fixing

« Back to merge proposal