DaanHoogland commented on a change in pull request #4438:
URL: https://github.com/apache/cloudstack/pull/4438#discussion_r662942437
##########
File path:
engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java
##########
@@ -411,14 +414,50 @@ public static void setValues(PreparedStatement
preparedStatement, Object... valu
}
+ @Override
+ public Ternary<Long, Long, Long> findCapacityByZoneAndHostTag(Long zoneId,
String hostTag) {
+ TransactionLegacy txn = TransactionLegacy.currentTxn();
+ PreparedStatement pstmt = null;
+ List<SummedCapacity> results = new ArrayList<SummedCapacity>();
+
+ StringBuilder allocatedSql = new
StringBuilder(LIST_ALLOCATED_CAPACITY_GROUP_BY_CAPACITY_AND_ZONE_PART1);
+ if (hostTag != null && ! hostTag.isEmpty()) {
+ allocatedSql.append("LEFT JOIN vm_template ON vm_template.id =
vi.vm_template_id ");
Review comment:
```suggestion
private static final String LEFT_JOIN_VMTEMPLATE = "LEFT JOIN
vm_template ON vm_template.id = vi.vm_template_id ";
```
up top and here
```suggestion
allocatedSql.append(LEFT_JOIN_VMTEMPLATE);
```
##########
File path:
engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java
##########
@@ -187,16 +188,18 @@
+
"from op_host_capacity capacity where cluster_id = ? and
capacity_type = ?;";
- private static final String
LIST_ALLOCATED_CAPACITY_GROUP_BY_CAPACITY_AND_ZONE = "SELECT v.data_center_id,
SUM(cpu) AS cpucore, " +
+ private static final String
LIST_ALLOCATED_CAPACITY_GROUP_BY_CAPACITY_AND_ZONE_PART1 = "SELECT
v.data_center_id, SUM(cpu) AS cpucore, " +
"SUM(cpu * speed) AS cpu, SUM(ram_size * 1024 * 1024) AS
memory " +
"FROM (SELECT vi.data_center_id, (CASE WHEN
ISNULL(service_offering.cpu) THEN custom_cpu.value ELSE service_offering.cpu
end) AS cpu, " +
"(CASE WHEN ISNULL(service_offering.speed) THEN
custom_speed.value ELSE service_offering.speed end) AS speed, " +
"(CASE WHEN ISNULL(service_offering.ram_size) THEN
custom_ram_size.value ELSE service_offering.ram_size end) AS ram_size " +
- "FROM (((vm_instance vi LEFT JOIN service_offering
ON(((vi.service_offering_id = service_offering.id))) " +
- "LEFT JOIN user_vm_details custom_cpu ON(((custom_cpu.vm_id =
vi.id) AND (custom_cpu.name = 'CpuNumber')))) " +
- "LEFT JOIN user_vm_details custom_speed
ON(((custom_speed.vm_id = vi.id) AND (custom_speed.name = 'CpuSpeed')))) " +
- "LEFT JOIN user_vm_details custom_ram_size
ON(((custom_ram_size.vm_id = vi.id) AND (custom_ram_size.name = 'memory')))) " +
- "WHERE ISNULL(vi.removed) AND vi.state NOT IN ('Destroyed',
'Error', 'Expunging')";
+ "FROM vm_instance vi LEFT JOIN service_offering
ON(((vi.service_offering_id = service_offering.id))) " +
+ "LEFT JOIN user_vm_details custom_cpu ON(((custom_cpu.vm_id =
vi.id) AND (custom_cpu.name = 'CpuNumber'))) " +
+ "LEFT JOIN user_vm_details custom_speed
ON(((custom_speed.vm_id = vi.id) AND (custom_speed.name = 'CpuSpeed'))) " +
+ "LEFT JOIN user_vm_details custom_ram_size
ON(((custom_ram_size.vm_id = vi.id) AND (custom_ram_size.name = 'memory'))) ";
+
+ private static final String
LIST_ALLOCATED_CAPACITY_GROUP_BY_CAPACITY_AND_ZONE_PART2 =
Review comment:
and then of course
```suggestion
private static final String WHERE_STATE_IS_NOT_DESTRUCTIVE =
```
or something like that
##########
File path: engine/schema/src/main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java
##########
@@ -802,6 +802,27 @@ public Long countByZoneAndState(long zoneId, State state) {
return customSearch(sc, null).get(0);
}
+ @Override
+ public Long countByZoneAndStateAndHostTag(long dcId, State state, String
hostTag) {
+ TransactionLegacy txn = TransactionLegacy.currentTxn();
+ String sql = "SELECT COUNT(1) FROM vm_instance vi "
+ + "JOIN service_offering so ON vi.service_offering_id=so.id "
+ + "JOIN vm_template vt ON vi.vm_template_id = vt.id "
+ + "WHERE vi.data_center_id= " + dcId + " AND vi.state = '" +
state + "' AND vi.removed IS NULL "
+ + "AND (so.host_tag='" + hostTag + "' OR vt.template_tag='" +
hostTag + "')";
Review comment:
Why not define this with prepared statement syntax? these parameters are
very suitable for that
```suggestion
String sql = "SELECT COUNT(1) FROM vm_instance vi "
+ "JOIN service_offering so ON vi.service_offering_id=so.id "
+ "JOIN vm_template vt ON vi.vm_template_id = vt.id "
+ "WHERE vi.data_center_id= ? AND vi.state = ? AND
vi.removed IS NULL "
+ "AND (so.host_tag = ? OR vt.template_tag = ? )";
```
and later assign `dcId`, `state`, `hostTag` and again `state`. It is saver
and if executed a second time in the same db engine also quicker.
##########
File path:
plugins/integrations/prometheus/src/main/java/org/apache/cloudstack/metrics/PrometheusExporterImpl.java
##########
@@ -115,6 +122,10 @@ private void addHostMetrics(final List<Item> metricsList,
final long dcId, final
int total = 0;
int up = 0;
int down = 0;
+ Map<String, Integer> total_hosts = new HashMap<>();
Review comment:
before you touched this method it was already 80 lines, can you please
refactor at least the new bits out into smaller chunks?
thanks
##########
File path:
engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java
##########
@@ -187,16 +188,18 @@
+
"from op_host_capacity capacity where cluster_id = ? and
capacity_type = ?;";
- private static final String
LIST_ALLOCATED_CAPACITY_GROUP_BY_CAPACITY_AND_ZONE = "SELECT v.data_center_id,
SUM(cpu) AS cpucore, " +
+ private static final String
LIST_ALLOCATED_CAPACITY_GROUP_BY_CAPACITY_AND_ZONE_PART1 = "SELECT
v.data_center_id, SUM(cpu) AS cpucore, " +
Review comment:
now we are touching this anyway,
```suggestion
private static final String
SELECT_ALLOCATED_CAPACITY_GROUP_BY_CAPACITY_AND_ZONE = "SELECT
v.data_center_id, SUM(cpu) AS cpucore, " +
```
is maybe a better name
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
