Lior Vernia has posted comments on this change. Change subject: engine: Persist total RX/TX in compatible hosts/VMs ......................................................................
Patch Set 8: (5 comments) http://gerrit.ovirt.org/#/c/36470/8/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/NetworkStatisticsBuilder.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/NetworkStatisticsBuilder.java: Line 76: stats.offset = offset; Line 77: stats.rate = null; Line 78: } else if (offset == null) { Line 79: // statistic reported for the first time - set to zero, set offset accordingly, and rate can't be computed Line 80: stats.current = 0L; > Why not- The rx/tx statistics are collected by the host since its boot, so when it's added to oVirt these counters won't be zero - and we do want to start from zero when the host is added. Line 81: stats.offset = -reported; Line 82: stats.rate = null; Line 83: } else { Line 84: stats.offset = offset; http://gerrit.ovirt.org/#/c/36470/8/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmAnalyzer.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VmAnalyzer.java: Line 754: // if rtl+pv it will get here 2 times (we take the max one) Line 755: if (firstTime) { Line 756: statsBuilder.updateExistingInterfaceStatistics(vmIface, ifStats); Line 757: } else { Line 758: vmIface.getStatistics().setReceiveRate(Math.max(vmIface.getStatistics().getReceiveRate(), > I'm not sure how can we get the same mac twice, but what about rx/txBytes a Old pv/rtl driver. But not supposed to be represented by dual NICs anymore, so there's no reason to keep maintaining this code... Line 759: ifStats.getStatistics().getReceiveRate())); Line 760: vmIface.getStatistics().setReceiveDropRate(Math.max(vmIface.getStatistics().getReceiveDropRate(), Line 761: ifStats.getStatistics().getReceiveDropRate())); Line 762: vmIface.getStatistics().setTransmitRate(Math.max(vmIface.getStatistics().getTransmitRate(), http://gerrit.ovirt.org/#/c/36470/8/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java: Line 890: } Line 891: Line 892: private static void extractInterfaceStatistics(Map<String, Object> dict, NetworkInterface<?> iface) { Line 893: NetworkStatistics stats = iface.getStatistics(); Line 894: stats.setReceiveRate(assignDoubleValueWithNullProtection(dict, VdsProperties.rx_rate)); > Shouldn't the rate extraction be done just for old versions? At this point we only store what's reported by vdsm, without implementing any logic. The version considerations happen in the stats builder. This code can be removed in 4.0, along with a lot of other code... Line 895: stats.setReceiveDropRate(assignDoubleValueWithNullProtection(dict, VdsProperties.rx_dropped)); Line 896: stats.setReceivedBytes(AssignLongValue(dict, VdsProperties.rx_total)); Line 897: stats.setTransmitRate(assignDoubleValueWithNullProtection(dict, VdsProperties.tx_rate)); Line 898: stats.setTransmitDropRate(assignDoubleValueWithNullProtection(dict, VdsProperties.tx_dropped)); http://gerrit.ovirt.org/#/c/36470/8/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java: Line 67: public static final String rx_total = "rx"; // in vm also Line 68: public static final String tx_dropped = "txDropped"; // in vm also Line 69: public static final String tx_rate = "txRate"; // in vm also Line 70: public static final String tx_total = "tx"; // in vm also Line 71: public static final String iface_status = "state"; // in vm also > Didn't find any use case of this field for vms. Done Line 72: public static final String sample_time = "sampleTime"; // in vm also Line 73: public static final String vm_active = "vmActive"; Line 74: public static final String vm_count = "vmCount"; Line 75: public static final String vm_migrating = "vmMigrating"; http://gerrit.ovirt.org/#/c/36470/8/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Cloner.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Cloner.java: Line 531: obj.setReceivedBytesOffset(instance.getReceivedBytesOffset()); Line 532: obj.setTransmitDropRate(instance.getTransmitDropRate()); Line 533: obj.setTransmitRate(instance.getTransmitRate()); Line 534: obj.setTransmittedBytes(instance.getTransmittedBytes()); Line 535: obj.setTransmittedBytesOffset(instance.getTransmittedBytesOffset()); > what about the sample time? You're right. Fixed in an earlier patchset, the one introducing the new fields to the NetworkStatistics entity - now that this logic sits in a copy constructor... Line 536: obj.setStatus(instance.getStatus()); Line 537: } Line 538: Line 539: private static VmNetworkStatistics cloneVmNetworkStatistics(VmNetworkStatistics instance) -- To view, visit http://gerrit.ovirt.org/36470 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3063bb36fb7e4d5a545d42c648091688ce1e6964 Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@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
