Greg Padgett has uploaded a new change for review. Change subject: broker: use direct I/O to read metadata ......................................................................
broker: use direct I/O to read metadata In some cases a host's filesystem cache may hold stale data from parts of the shared metadata file not updated by that host. This can lead to the host falsely concluding that other hosts are down. Using direct I/O for the read operations avoids this problem by fetching the latest data directly from the NFS server. Change-Id: I296977000ffa3fff3f9391f93a8b4f3f519eae4e Bug-Url: https://bugzilla.redhat.com/1014241 Signed-off-by: Greg Padgett <[email protected]> --- M ovirt_hosted_engine_ha/broker/constants.py.in M ovirt_hosted_engine_ha/broker/storage_broker.py 2 files changed, 25 insertions(+), 26 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-hosted-engine-ha refs/changes/60/19760/1 diff --git a/ovirt_hosted_engine_ha/broker/constants.py.in b/ovirt_hosted_engine_ha/broker/constants.py.in index 1ec3fff..1ca0b76 100644 --- a/ovirt_hosted_engine_ha/broker/constants.py.in +++ b/ovirt_hosted_engine_ha/broker/constants.py.in @@ -38,3 +38,4 @@ MD_EXTENSION = 'metadata' HOST_SEGMENT_BYTES = 4096 MAX_HOST_ID_SCAN = 64 +USE_DIRECT_IO = True diff --git a/ovirt_hosted_engine_ha/broker/storage_broker.py b/ovirt_hosted_engine_ha/broker/storage_broker.py index b1c3507..c6e7e78 100644 --- a/ovirt_hosted_engine_ha/broker/storage_broker.py +++ b/ovirt_hosted_engine_ha/broker/storage_broker.py @@ -57,33 +57,31 @@ """ self._log.info("Getting stats for service %s from %s", service_type, storage_dir) - d = {} - with self._storage_access_lock: - path = os.path.join(storage_dir, self._get_filename(service_type)) - f = None - try: - f = io.open(path, "r+b") - host_id = 0 - for data in iter( - lambda: f.read(constants.HOST_SEGMENT_BYTES), - '' - ): - # TODO it would be better if this was configurable - if host_id > constants.MAX_HOST_ID_SCAN: - break - if data and data[0] != '\0': - d[host_id] = data - host_id += 1 - except IOError as e: - self._log.error("Failed to read metadata from %s", - path, exc_info=True) - raise RequestError("failed to read metadata: {0}" - .format(str(e))) - finally: - if f: - f.close() + path = os.path.join(storage_dir, self._get_filename(service_type)) - return d + # Use direct I/O if possible, to avoid the local filesystem cache + # from hiding metadata file updates from other hosts. For NFS, we + # don't have to worry about alignment; see man open(2) for details. + # TODO it would be better if this was configurable + direct_flag = os.O_DIRECT if constants.USE_DIRECT_IO else 0 + + bs = constants.HOST_SEGMENT_BYTES + # TODO it would be better if this was configurable + read_size = bs * (constants.MAX_HOST_ID_SCAN + 1) + + try: + with self._storage_access_lock: + f = os.open(path, direct_flag | os.O_RDONLY) + data = os.read(f, read_size) + os.close(f) + except IOError as e: + self._log.error("Failed to read metadata from %s", + path, exc_info=True) + raise RequestError("failed to read metadata: {0}".format(str(e))) + + return dict(((i / bs, data[i:i+bs]) + for i in xrange(0, len(data), bs) + if data[i] != '\0')) def put_stats(self, storage_dir, service_type, host_id, data): """ -- To view, visit http://gerrit.ovirt.org/19760 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I296977000ffa3fff3f9391f93a8b4f3f519eae4e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-hosted-engine-ha Gerrit-Branch: master Gerrit-Owner: Greg Padgett <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
