Skip to content

Commit 51daef2

Browse files
SCadilhacSCadilhac
committed
perf(db): ⚡ improve performance of device tasks by removing useless requests
1 parent d7010c8 commit 51daef2

File tree

11 files changed

+70
-102
lines changed

11 files changed

+70
-102
lines changed

‎src/main/java/net/netshot/netshot/database/Database.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ public static void init() {
243243
// serviceProperties.setProperty(AvailableSettings.SHOW_SQL, "true");
244244
serviceProperties.setProperty(AvailableSettings.FORMAT_SQL, "true");
245245
// Dates/times stored in UTC in the DB, without timezone, up to Java to convert to server local time
246-
serviceProperties.setProperty("hibernate.jdbc.time_zone", "UTC");
246+
serviceProperties.setProperty(AvailableSettings.JDBC_TIME_ZONE, "UTC");
247247

248248
// Use the custom Multi Tenant connection provider (to handle read vs read-write connections)
249249
final CustomConnectionProvider connectionProvider = new CustomConnectionProvider();

‎src/main/java/net/netshot/netshot/device/Config.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.LinkedList;
2525
import java.util.List;
2626
import java.util.Map;
27+
import java.util.Objects;
2728
import java.util.Set;
2829

2930
import jakarta.persistence.CascadeType;
@@ -188,4 +189,17 @@ public ConfigAttribute getAttribute(String name) {
188189
return null;
189190
}
190191

192+
@Override
193+
public int hashCode() {
194+
return Objects.hash(id);
195+
}
196+
197+
@Override
198+
public boolean equals(Object obj) {
199+
if (this == obj) return true;
200+
if (!(obj instanceof Config)) return false;
201+
Config other = (Config) obj;
202+
return id == other.id;
203+
}
204+
191205
}

‎src/main/java/net/netshot/netshot/device/DeviceGroup.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import jakarta.xml.bind.annotation.XmlRootElement;
4242

4343
import org.hibernate.Session;
44-
import org.hibernate.annotations.Cascade;
4544
import org.hibernate.annotations.NaturalId;
4645
import org.hibernate.annotations.OnDelete;
4746
import org.hibernate.annotations.OnDeleteAction;

‎src/main/java/net/netshot/netshot/device/NetworkInterface.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import jakarta.persistence.ElementCollection;
2727
import jakarta.persistence.Embedded;
2828
import jakarta.persistence.Entity;
29-
import jakarta.persistence.FetchType;
3029
import jakarta.persistence.GeneratedValue;
3130
import jakarta.persistence.GenerationType;
3231
import jakarta.persistence.Id;
@@ -84,15 +83,15 @@ public class NetworkInterface {
8483

8584
/** The ip4 addresses. */
8685
@Getter(onMethod=@__({
87-
@ElementCollection(fetch = FetchType.EAGER), @Fetch(FetchMode.SELECT),
86+
@ElementCollection, @Fetch(FetchMode.SUBSELECT),
8887
@XmlElement, @JsonView(DefaultView.class)
8988
}))
9089
@Setter
9190
protected Set<Network4Address> ip4Addresses = new HashSet<>();
9291

9392
/** The ip6 addresses. */
9493
@Getter(onMethod=@__({
95-
@ElementCollection(fetch = FetchType.EAGER), @Fetch(FetchMode.SELECT),
94+
@ElementCollection, @Fetch(FetchMode.SUBSELECT),
9695
@XmlElement, @JsonView(DefaultView.class)
9796
}))
9897
@Setter

‎src/main/java/net/netshot/netshot/device/attribute/ConfigAttribute.java

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
import net.netshot.netshot.device.Config;
3838
import net.netshot.netshot.rest.RestViews.DefaultView;
3939

40+
import java.util.Objects;
41+
4042
import com.fasterxml.jackson.annotation.JsonSubTypes;
4143
import com.fasterxml.jackson.annotation.JsonSubTypes.Type;
4244

@@ -99,29 +101,15 @@ public ConfigAttribute(Config config, String name) {
99101

100102
@Override
101103
public int hashCode() {
102-
final int prime = 31;
103-
int result = 1;
104-
result = prime * result + ((name == null) ? 0 : name.hashCode());
105-
return result;
104+
return Objects.hash(name, config);
106105
}
107106

108107
@Override
109108
public boolean equals(Object obj) {
110-
if (this == obj)
111-
return true;
112-
if (obj == null)
113-
return false;
114-
if (!(obj instanceof ConfigAttribute))
115-
return false;
109+
if (this == obj) return true;
110+
if (!(obj instanceof ConfigAttribute)) return false;
116111
ConfigAttribute other = (ConfigAttribute) obj;
117-
if (name == null) {
118-
if (other.name != null)
119-
return false;
120-
}
121-
else if (!name.equals(other.name))
122-
return false;
123-
return true;
112+
return Objects.equals(name, other.name) && Objects.equals(config, other.config);
124113
}
125114

126-
127115
}

‎src/main/java/net/netshot/netshot/device/attribute/DeviceAttribute.java

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
import net.netshot.netshot.device.Device;
3838
import net.netshot.netshot.rest.RestViews.DefaultView;
3939

40+
import java.util.Objects;
41+
4042
import org.hibernate.annotations.OnDelete;
4143
import org.hibernate.annotations.OnDeleteAction;
4244

@@ -93,28 +95,15 @@ public DeviceAttribute(Device device, String name) {
9395
public abstract Object getData();
9496

9597
@Override
96-
public final int hashCode() {
97-
final int prime = 31;
98-
int result = 1;
99-
result = prime * result + ((name == null) ? 0 : name.hashCode());
100-
return result;
98+
public int hashCode() {
99+
return Objects.hash(name, device);
101100
}
102101

103102
@Override
104-
public final boolean equals(Object obj) {
105-
if (this == obj)
106-
return true;
107-
if (obj == null)
108-
return false;
109-
if (!(obj instanceof DeviceAttribute))
110-
return false;
103+
public boolean equals(Object obj) {
104+
if (this == obj) return true;
105+
if (!(obj instanceof DeviceAttribute)) return false;
111106
DeviceAttribute other = (DeviceAttribute) obj;
112-
if (name == null) {
113-
if (other.name != null)
114-
return false;
115-
}
116-
else if (!name.equals(other.name))
117-
return false;
118-
return true;
107+
return Objects.equals(name, other.name) && Objects.equals(device, other.device);
119108
}
120109
}

‎src/main/java/net/netshot/netshot/device/script/CliScript.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -161,15 +161,16 @@ public void connectRun(Session session, Device device, Set<DeviceCredentialSet>
161161
if (address == null) {
162162
address = device.getMgmtAddress();
163163
}
164-
Set<DeviceCredentialSet> credentialSets = oneTimeCredentialSets;
165-
if (credentialSets == null) {
166-
credentialSets = device.getCredentialSets();
164+
Set<DeviceCredentialSet> credentialSets = new HashSet<>();
165+
if (oneTimeCredentialSets != null) {
166+
credentialSets.addAll(oneTimeCredentialSets);
167167
}
168-
169-
if (device.getSpecificCredentialSet() != null) {
170-
credentialSets = new HashSet<DeviceCredentialSet>();
168+
if (credentialSets.size() == 0 && device.getSpecificCredentialSet() != null) {
171169
credentialSets.add(device.getSpecificCredentialSet());
172170
}
171+
if (credentialSets.size() == 0) {
172+
credentialSets.addAll(device.getCredentialSets());
173+
}
173174

174175
int sshPort = device.getSshPort();
175176
if (sshPort == 0) {

‎src/main/java/net/netshot/netshot/work/tasks/CheckComplianceTask.java

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,6 @@ public CheckComplianceTask(Device device, String comments, String author) {
8080
this.device = device;
8181
}
8282

83-
/* (non-Javadoc)
84-
* @see net.netshot.netshot.work.Task#prepare()
85-
*/
86-
@Override
87-
public void prepare(Session session) {
88-
Hibernate.initialize(device);
89-
}
90-
9183
/* (non-Javadoc)
9284
* @see net.netshot.netshot.work.Task#run()
9385
*/
@@ -100,23 +92,27 @@ public void run() {
10092
this.status = Status.CANCELLED;
10193
return;
10294
}
103-
this.info(String.format("Check compliance task for device %s (%s).",
104-
device.getName(), device.getMgmtAddress().getIp()));
95+
10596
Session session = Database.getSession();
10697
try {
10798
session.beginTransaction();
99+
// Delete previous results
108100
session
109101
.createMutationQuery("delete from CheckResult c where c.key.device.id = :id")
110102
.setParameter("id", this.device.getId())
111103
.executeUpdate();
112-
session.refresh(this.device);
104+
// Start over from a fresh device from DB
105+
device = session.get(Device.class, device.getId());
106+
this.info(String.format("Check compliance task for device %s (%s).",
107+
device.getName(), device.getMgmtAddress().getIp()));
113108
if (this.device.getLastConfig() == null) {
114109
log.info("Task {}. Unable to fetch the device with its last config... has it been captured at least once?",
115110
this.getId());
116111
throw new Exception("No last config for this device. Has it been captured at least once?");
117112
}
118113
List<Policy> policies = session
119-
.createQuery("select distinct p from Policy p join p.targetGroups g join g.cachedMemberships dm with dm.key.device.id = :id", Policy.class)
114+
.createQuery("select distinct p from Policy p join p.targetGroups g " +
115+
"join g.cachedMemberships dm with dm.key.device.id = :id", Policy.class)
120116
.setParameter("id", this.device.getId())
121117
.list();
122118

@@ -129,7 +125,9 @@ public void run() {
129125
session.persist(this.device);
130126
session.flush();
131127
List<SoftwareRule> softwareRules = session
132-
.createQuery("select sr from SoftwareRule sr where (sr.targetGroup is null) or sr.targetGroup in (select g from DeviceGroup g join g.cachedMemberships dm with dm.key.device.id = :id) order by sr.priority asc", SoftwareRule.class)
128+
.createQuery("select sr from SoftwareRule sr where (sr.targetGroup is null) or " +
129+
"sr.targetGroup in (select g from DeviceGroup g join g.cachedMemberships dm " +
130+
"with dm.key.device.id = :id) order by sr.priority asc", SoftwareRule.class)
133131
.setParameter("id", this.device.getId())
134132
.list();
135133
device.setSoftwareLevel(ConformanceLevel.UNKNOWN);

‎src/main/java/net/netshot/netshot/work/tasks/RunDeviceScriptTask.java

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import net.netshot.netshot.rest.RestViews.DefaultView;
3939
import net.netshot.netshot.work.Task;
4040

41-
import org.hibernate.Hibernate;
4241
import org.hibernate.Session;
4342
import org.hibernate.annotations.JdbcTypeCode;
4443
import org.hibernate.annotations.OnDelete;
@@ -85,13 +84,13 @@ public class RunDeviceScriptTask extends Task implements DeviceBasedTask {
8584
private Map<String, String> userInputValues = null;
8685

8786
/**
88-
* Instantiates a new take snapshot task.
87+
* Instantiates a new RunDeviceScriptTask task.
8988
*/
9089
protected RunDeviceScriptTask() {
9190
}
9291

9392
/**
94-
* Instantiates a new take snapshot task.
93+
* Instantiates a new RunDeviceScriptTask task.
9594
*
9695
* @param device the device
9796
* @param comments the comments
@@ -104,12 +103,6 @@ public RunDeviceScriptTask(Device device, String script, DeviceDriver driver, St
104103
this.deviceDriver = driver.getName();
105104
}
106105

107-
@Override
108-
public void prepare(Session session) {
109-
Hibernate.initialize(this.getDevice());
110-
}
111-
112-
113106
@Override
114107
public void run() {
115108
log.debug("Task {}. Starting script task for device {}.", this.getId(),
@@ -119,14 +112,15 @@ public void run() {
119112
this.status = Status.CANCELLED;
120113
return;
121114
}
122-
this.info(String.format("Run script task for device %s (%s).",
123-
device.getName(), device.getMgmtAddress().getIp()));
124115

125116
JsCliScript cliScript = null;
126117
Session session = Database.getSession();
127118
try {
128119
session.beginTransaction();
129-
session.refresh(device);
120+
// Start over from a fresh device from DB
121+
device = session.get(Device.class, device.getId());
122+
this.info(String.format("Run script task for device %s (%s).",
123+
device.getName(), device.getMgmtAddress().getIp()));
130124
if (deviceDriver == null || !deviceDriver.equals(device.getDriver())) {
131125
log.trace("Task {}. The script doesn't apply to the driver of the device.", this.getId());
132126
this.error("The script doesn't apply to the driver of the device.");

‎src/main/java/net/netshot/netshot/work/tasks/RunDiagnosticsTask.java

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
import lombok.Setter;
3535
import lombok.extern.slf4j.Slf4j;
3636

37-
import org.hibernate.Hibernate;
3837
import org.hibernate.Session;
3938
import org.hibernate.annotations.OnDelete;
4039
import org.hibernate.annotations.OnDeleteAction;
@@ -114,14 +113,6 @@ public RunDiagnosticsTask(Device device, String comments, String author, boolean
114113
this.device = device;
115114
this.dontCheckCompliance = dontCheckCompliance;
116115
}
117-
118-
/* (non-Javadoc)
119-
* @see net.netshot.netshot.work.Task#prepare()
120-
*/
121-
@Override
122-
public void prepare(Session session) {
123-
Hibernate.initialize(this.device);
124-
}
125116

126117
@Override
127118
@XmlElement @JsonView(DefaultView.class)
@@ -163,16 +154,16 @@ public void run() {
163154
this.status = Status.CANCELLED;
164155
return;
165156
}
166-
this.trace(String.format("Run diagnostic task for device %s (%s).",
167-
device.getName(), device.getMgmtAddress().getIp()));
168157
boolean locked = false;
169-
170-
RunDiagnosticCliScript cliScript = null;
171158

172159
Session session = Database.getSession();
160+
RunDiagnosticCliScript cliScript = null;
173161
try {
174162
session.beginTransaction();
175-
session.refresh(device);
163+
// Start over from a fresh device from DB
164+
device = session.get(Device.class, device.getId());
165+
this.trace(String.format("Run diagnostic task for device %s (%s).",
166+
device.getName(), device.getMgmtAddress().getIp()));
176167
if (device.getStatus() != Device.Status.INPRODUCTION) {
177168
log.trace("Task {}. Device not INPRODUCTION, stopping the diagnostic task.", this.getId());
178169
this.warn("The device is not enabled (not in production).");
@@ -188,8 +179,8 @@ public void run() {
188179
}
189180

190181
List<Diagnostic> diagnostics = session.createQuery(
191-
"select dg from Diagnostic dg where dg.enabled = :enabled and " +
192-
"dg.targetGroup in (select gm.key.group from DeviceGroupMembership gm where gm.key.device = :device)",
182+
"select dg from Diagnostic dg join fetch dg.targetGroup tg where dg.enabled = :enabled and " +
183+
"tg in (select gm.key.group from DeviceGroupMembership gm where gm.key.device = :device)",
193184
Diagnostic.class)
194185
.setParameter("device", device)
195186
.setParameter("enabled", true)
@@ -201,6 +192,9 @@ public void run() {
201192
session.merge(device);
202193
session.getTransaction().commit();
203194
}
195+
else {
196+
this.info("No diagnostic was found that apply to this device.");
197+
}
204198
this.status = Status.SUCCESS;
205199

206200
}
@@ -223,7 +217,7 @@ public void run() {
223217
}
224218
finally {
225219
try {
226-
if (this.debugEnabled) {
220+
if (this.debugEnabled && cliScript != null) {
227221
this.debugLog = new DebugLog(cliScript.getPlainCliLog());
228222
}
229223
}

0 commit comments

Comments
 (0)