Alon Bar-Lev has posted comments on this change. Change subject: fklsnr: Introduce standalone fence_kdump listener ......................................................................
Patch Set 11: (6 comments) http://gerrit.ovirt.org/#/c/27201/11/packaging/services/ovirt-fence-kdump-listener/db.py File packaging/services/ovirt-fence-kdump-listener/db.py: Line 236: def update_vds_kdump_status(self, vds_id, status, address): Line 237: return self._db_mgr.call_procedure( Line 238: name='UpsertKdumpStatus', Line 239: args=( Line 240: vds_id, # v_vds_id > It's very hard to parse errors from db exception. That's why I prefer to fi return # of lines modified or a value, not an error. Line 241: status, # v_status Line 242: json.dumps(address), # v_address Line 243: ), Line 244: ) http://gerrit.ovirt.org/#/c/27201/11/packaging/services/ovirt-fence-kdump-listener/listener.py File packaging/services/ovirt-fence-kdump-listener/listener.py: Line 126: _( Line 127: "Discarding invalid message '{msg}' from address " Line 128: "'{address}'." Line 129: ).format( Line 130: msg=binascii.hexlify(message), > Is there any difference I should know? I found this on internet when I sear less dependencies makes me happy. Line 131: address=entry['address'][0], Line 132: ) Line 133: ) Line 134: Line 129: ).format( Line 130: msg=binascii.hexlify(message), Line 131: address=entry['address'][0], Line 132: ) Line 133: ) > I need to stop processing if message is invalid. You didn't like that I cal you should have: try: function body, throw an excpetion if error catch E: move entry to CLOSED state all within the same function Line 134: Line 135: # message is valid, update timestamp Line 136: entry['updated'] = datetime.datetime.utcnow() Line 137: Line 211: if session['status'] == 'finished': Line 212: del self._sessions[session['address']] Line 213: Line 214: def _load_sessions(self): Line 215: self._lastDbSync = datetime.datetime.utcnow() > Well, it's needed only at startup now (if we drop the vdc_options updates, we need this at fist database connection establish, so we can start without database Line 216: for record in self._dao.get_unfinished_sessions(): Line 217: session = { Line 218: 'status': record['status'], Line 219: 'address': record['address'], Line 220: 'updated': self._lastDbSync, Line 221: } Line 222: self._sessions[session['address']] = session Line 223: Line 224: def _db_sync(self): > Well, I wanted session changes to be written into db asap, so in this case why? what's the rush? the entire point was to relief load from database, even if we get 100000000 packets per second we persist once in 5 seconds or so. Line 225: if self._db_manager.validate_connection(): Line 226: try: Line 227: self._save_heartbeat() Line 228: self._save_sessions() Line 268: entry=entry, Line 269: message=data, Line 270: ) Line 271: except InvalidMessage as e: Line 272: self.logger.error(e) > I wanted to alert sysadmin that someone is sending non fence_kdump traffic yes, but you end up with filling his filesystem, causing server to halt. Line 273: Line 274: # try to sync with db Line 275: self._db_sync() Line 276: -- To view, visit http://gerrit.ovirt.org/27201 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieec3bad47bbba860a52a9ff4e2eb7f61277f4e36 Gerrit-PatchSet: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
