Skip to content

Commit e1cb5d7

Browse files
anheftianhefti
committed
SEBSP-210 very short term improvements
catch data source health check, add criticalRequestBucket just for testing, increase spring.datasource.hikari.maximumPoolSize from 10 to 50
1 parent 922d5e6 commit e1cb5d7

File tree

5 files changed

+70
-49
lines changed

5 files changed

+70
-49
lines changed

‎src/main/java/ch/ethz/seb/sps/server/datalayer/dao/ScreenshotDataDAO.java‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,6 @@ public interface ScreenshotDataDAO extends EntityDAO<ScreenshotData, ScreenshotD
4040

4141
Result<ScreenshotDataRecord> getLatest(String sessionUUID);
4242

43-
// @Transactional(readOnly = true)
44-
// default Result<Map<String, ScreenshotDataRecord>> allLatestOf(final String sessionUUIDList) {
45-
// return Result.tryCatch(() -> {
46-
//
47-
// final List<String> ids = sessionUUIDList.contains(Constants.LIST_SEPARATOR)
48-
// ? Arrays.asList(StringUtils.split(sessionUUIDList, Constants.LIST_SEPARATOR))
49-
// : Arrays.asList(sessionUUIDList);
50-
//
51-
// return allLatestIn(ids).getOrThrow();
52-
// });
53-
// }
54-
5543
Result<Map<String, ScreenshotDataRecord>> allLatestIn(List<String> sessionUUIDs);
5644

5745
Result<Long> save(

‎src/main/java/ch/ethz/seb/sps/server/datalayer/dao/impl/ScreenshotDataDAOBatis.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ public Result<Collection<ScreenshotData>> allOfSession(final String sessionUUID)
126126
@Override
127127
@Transactional(readOnly = true)
128128
public Result<ScreenshotDataRecord> getAt(final String sessionUUID, final Long at) {
129+
// TODO: this seems to produce performance issues when table is huge. Try to optimize query by reducing the time frame here
129130
return Result.tryCatch(() -> {
130131
ScreenshotDataRecord record = SelectDSL
131132
.selectWithMapper(this.screenshotDataRecordMapper::selectOne,

‎src/main/java/ch/ethz/seb/sps/server/servicelayer/impl/SessionServiceHealthControlImpl.java‎

Lines changed: 41 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -97,18 +97,23 @@ public int getStoreHealthIndicator() {
9797

9898
@Override
9999
public int getDataSourceHealthIndicator() {
100-
if (this.dataSource instanceof HikariDataSource) {
101-
final HikariDataSource hds = (HikariDataSource) this.dataSource;
102-
final HikariPoolMXBean hikariPoolMXBean = hds.getHikariPoolMXBean();
103-
final int idleConnections = hikariPoolMXBean.getIdleConnections();
104-
final int activeConnections = hikariPoolMXBean.getActiveConnections();
100+
try {
101+
if (this.dataSource instanceof HikariDataSource) {
102+
final HikariDataSource hds = (HikariDataSource) this.dataSource;
103+
final HikariPoolMXBean hikariPoolMXBean = hds.getHikariPoolMXBean();
104+
final int idleConnections = hikariPoolMXBean.getIdleConnections();
105+
final int activeConnections = hikariPoolMXBean.getActiveConnections();
106+
}
107+
final Health health = this.dataSourceHealthIndicator.getHealth(true);
108+
final Map<String, Object> details = health.getDetails();
109+
// TODO check what is in details
110+
final Status status = health.getStatus();
111+
// TODO
112+
return 0;
113+
} catch (Exception e) {
114+
log.warn("Failed to check data source health: {}", e.getMessage());
115+
return 10;
105116
}
106-
final Health health = this.dataSourceHealthIndicator.getHealth(true);
107-
final Map<String, Object> details = health.getDetails();
108-
// TODO check what is in details
109-
final Status status = health.getStatus();
110-
// TODO
111-
return 0;
112117
}
113118

114119
@Override
@@ -117,33 +122,34 @@ public int getOverallLoadIndicator() {
117122
return simulateHealthIssue;
118123
}
119124

120-
final int uploadHealthIndicator = getUploadHealthIndicator();
121-
final int downloadHealthIndicator = getDownloadHealthIndicator();
122-
final int storeHealthIndicator = getStoreHealthIndicator();
123-
final int dataSourceHealthIndicator = getDataSourceHealthIndicator();
124125

125-
if (log.isDebugEnabled()) {
126-
long now = Utils.getMillisecondsNow();
127-
if (now - debug_log_last_log_time > debug_log_time_interval) {
128-
debug_log_last_log_time = now;
129-
if (uploadHealthIndicator > 0) {
130-
log.info("uploadHealthIndicator: {}", uploadHealthIndicator);
131-
}
132-
if (downloadHealthIndicator > 0) {
133-
log.info("downloadHealthIndicator: {}", downloadHealthIndicator);
134-
}
135-
if (storeHealthIndicator > 0) {
136-
log.info("storeHealthIndicator: {}", storeHealthIndicator);
137-
}
138-
if (dataSourceHealthIndicator > 0) {
139-
log.info("dataSourceHealthIndicator: {}", dataSourceHealthIndicator);
126+
final int uploadHealthIndicator = getUploadHealthIndicator();
127+
final int downloadHealthIndicator = getDownloadHealthIndicator();
128+
final int storeHealthIndicator = getStoreHealthIndicator();
129+
final int dataSourceHealthIndicator = getDataSourceHealthIndicator();
130+
131+
if (log.isDebugEnabled()) {
132+
long now = Utils.getMillisecondsNow();
133+
if (now - debug_log_last_log_time > debug_log_time_interval) {
134+
debug_log_last_log_time = now;
135+
if (uploadHealthIndicator > 0) {
136+
log.info("uploadHealthIndicator: {}", uploadHealthIndicator);
137+
}
138+
if (downloadHealthIndicator > 0) {
139+
log.info("downloadHealthIndicator: {}", downloadHealthIndicator);
140+
}
141+
if (storeHealthIndicator > 0) {
142+
log.info("storeHealthIndicator: {}", storeHealthIndicator);
143+
}
144+
if (dataSourceHealthIndicator > 0) {
145+
log.info("dataSourceHealthIndicator: {}", dataSourceHealthIndicator);
146+
}
140147
}
141148
}
142-
}
143-
144-
return Math.max(
145-
Math.max(uploadHealthIndicator, downloadHealthIndicator),
146-
Math.max(storeHealthIndicator, dataSourceHealthIndicator));
149+
150+
return Math.max(
151+
Math.max(uploadHealthIndicator, downloadHealthIndicator),
152+
Math.max(storeHealthIndicator, dataSourceHealthIndicator));
147153
}
148154

149155
// map size between THREAD_POOL_SIZE_INDICATOR_MAP_MIN ... THREAD_POOL_SIZE_INDICATOR_MAP_MAX to 0 ... HEALTH_INDICATOR_MAX

‎src/main/java/ch/ethz/seb/sps/server/weblayer/AdminProctorController.java‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import java.util.*;
1313
import java.util.concurrent.CompletableFuture;
1414
import java.util.concurrent.Executor;
15+
import java.util.concurrent.atomic.AtomicInteger;
1516
import java.util.stream.Collectors;
1617

1718
import ch.ethz.seb.sps.domain.model.EntityType;
@@ -74,6 +75,8 @@ public class AdminProctorController {
7475
private final PaginationService paginationService;
7576
private final GroupingService groupingService;
7677
private final UserService userService;
78+
79+
private AtomicInteger criticalRequestBucket = new AtomicInteger(10);
7780

7881
public AdminProctorController(
7982
final GroupDAO groupDAO,
@@ -221,12 +224,23 @@ public ScreenshotsInGroupData getSessionsByGroup(
221224
@RequestParam(name = ScreenshotsInGroupData.ATTR_SORT_ORDER, required = false) final PageSortOrder sortOrder,
222225
@RequestParam(name = "filterCriteria", required = false) final MultiValueMap<String, String> filterCriteria,
223226
final HttpServletRequest request) {
224-
227+
228+
// TODO test this criticalRequestBucket strategy to prevent to many not responded requests
229+
// that possibly stuck on DB level
230+
if (criticalRequestBucket.get() <= 0) {
231+
if (log.isDebugEnabled()) {
232+
log.warn("criticalRequestBucket empty! This will deny request in the future");
233+
}
234+
}
235+
criticalRequestBucket.decrementAndGet();
236+
225237
this.proctoringService.checkMonitoringAccess(groupUUID);
226238

227239
final FilterMap filterMap = new FilterMap(filterCriteria, request.getQueryString());
228240
return this.proctoringService
229241
.getSessionsByGroup(groupUUID, pageNumber, pageSize, sortBy, sortOrder, filterMap)
242+
.onError(e -> { criticalRequestBucket.incrementAndGet(); })
243+
.onSuccess(r -> { criticalRequestBucket.incrementAndGet(); })
230244
.getOrThrow();
231245
}
232246

@@ -278,6 +292,15 @@ public ScreenshotViewData getScreenshotViewData(
278292

279293
this.proctoringService.checkMonitoringSessionAccess(sessionUUID);
280294

295+
// TODO test this criticalRequestBucket strategy to prevent to many not responded requests
296+
// that possibly stuck on DB level
297+
if (criticalRequestBucket.get() <= 0) {
298+
if (log.isDebugEnabled()) {
299+
log.warn("criticalRequestBucket empty! This will deny request in the future");
300+
}
301+
}
302+
criticalRequestBucket.decrementAndGet();
303+
281304
Long ts = null;
282305
if (StringUtils.isNotBlank(timestamp)) {
283306
try {
@@ -289,6 +312,8 @@ public ScreenshotViewData getScreenshotViewData(
289312

290313
return this.proctoringService
291314
.getRecordedImageDataAt(sessionUUID, ts)
315+
.onError(e -> { criticalRequestBucket.incrementAndGet(); })
316+
.onSuccess(r -> { criticalRequestBucket.incrementAndGet(); })
292317
.getOrThrow();
293318
}
294319

‎src/main/resources/config/application.properties‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ spring.datasource.hikari.initializationFailTimeout=3000
5050
spring.datasource.hikari.connectionTimeout=30000
5151
spring.datasource.hikari.idleTimeout=600000
5252
spring.datasource.hikari.maxLifetime=1800000
53-
spring.datasource.hikari.maximumPoolSize=10
53+
spring.datasource.hikari.maximumPoolSize=50
54+
spring.datasource.hikari.leakDetectionThreshold=10000
5455
spring.cache.jcache.config=classpath:config/ehcache.xml
5556

5657
### webservice security

0 commit comments

Comments
 (0)