weizhouapache commented on code in PR #7132:
URL: https://github.com/apache/cloudstack/pull/7132#discussion_r1086613497
##########
api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java:
##########
@@ -18,9 +18,31 @@
import com.cloud.utils.component.Manager;
import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint;
+import org.apache.cloudstack.framework.config.ConfigKey;
public interface ConsoleAccessManager extends Manager {
+ ConfigKey<Boolean> ConsoleSessionCleanupEnabled = new
ConfigKey<>("Advanced", Boolean.class,
+ "console.session.cleanup.enabled",
+ "true",
+ "Enables/disables the console session cleanup thread.",
+ true,
+ ConfigKey.Scope.Global);
+
+ ConfigKey<Integer> ConsoleSessionCleanupRetentionDays = new
ConfigKey<>("Advanced", Integer.class,
+ "console.session.cleanup.retention.days",
Review Comment:
maybe both in hours ?
##########
api/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManager.java:
##########
@@ -18,9 +18,31 @@
import com.cloud.utils.component.Manager;
import org.apache.cloudstack.api.command.user.consoleproxy.ConsoleEndpoint;
+import org.apache.cloudstack.framework.config.ConfigKey;
public interface ConsoleAccessManager extends Manager {
+ ConfigKey<Boolean> ConsoleSessionCleanupEnabled = new
ConfigKey<>("Advanced", Boolean.class,
+ "console.session.cleanup.enabled",
+ "true",
+ "Enables/disables the console session cleanup thread.",
+ true,
Review Comment:
If we change the values, we need to restart cloudstack management server,
right ?
if yes, these configurations are not dynamic.
##########
engine/schema/src/main/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java:
##########
@@ -19,11 +19,24 @@
package com.cloud.vm.dao;
+import com.cloud.utils.db.SearchBuilder;
+import com.cloud.utils.db.SearchCriteria;
import com.cloud.vm.ConsoleSessionVO;
import com.cloud.utils.db.GenericDaoBase;
+import java.util.Date;
+import java.util.List;
+
public class ConsoleSessionDaoImpl extends GenericDaoBase<ConsoleSessionVO,
Long> implements ConsoleSessionDao {
+ private final SearchBuilder<ConsoleSessionVO> SearchByRemovedDate;
+
+ public ConsoleSessionDaoImpl() {
+ SearchByRemovedDate = createSearchBuilder();
+ SearchByRemovedDate.and("removed",
SearchByRemovedDate.entity().getRemoved(), SearchCriteria.Op.NNULL);
Review Comment:
is this an issue that two conditions have the same key "removed" ?
may be better to rename key in line 36 to "removedIsNotNull" (just an
example)
##########
server/src/main/java/org/apache/cloudstack/consoleproxy/ConsoleAccessManagerImpl.java:
##########
@@ -100,9 +111,53 @@ public class ConsoleAccessManagerImpl extends ManagerBase
implements ConsoleAcce
@Override
public boolean configure(String name, Map<String, Object> params) throws
ConfigurationException {
ConsoleAccessManagerImpl.secretKeysManager = keysManager;
+ executorService = Executors.newScheduledThreadPool(1, new
NamedThreadFactory("ConsoleSession-Scavenger"));
return super.configure(name, params);
}
+ @Override
+ public boolean start() {
+ Integer consoleCleanupInterval =
ConsoleAccessManager.ConsoleSessionCleanupInterval.value();
+ Boolean consoleCleanupEnabled =
ConsoleAccessManager.ConsoleSessionCleanupEnabled.value();
+ if (BooleanUtils.isTrue(consoleCleanupEnabled)) {
+ s_logger.info(String.format("The ConsoleSessionCleanupTask will
run every %s hours", consoleCleanupInterval));
+ executorService.scheduleWithFixedDelay(new
ConsoleSessionCleanupTask(), consoleCleanupInterval, consoleCleanupInterval,
TimeUnit.HOURS);
+ }
+ return true;
+ }
+
+ public class ConsoleSessionCleanupTask extends ManagedContextRunnable {
+ @Override
+ protected void runInContext() {
+ final GlobalLock gcLock =
GlobalLock.getInternLock("ConsoleSession.Cleanup.Lock");
+ try {
+ if (gcLock.lock(3)) {
+ try {
+ reallyRun();
+ } finally {
+ gcLock.unlock();
+ }
+ }
+ } finally {
+ gcLock.releaseRef();
+ }
+ }
+
+ private void reallyRun() {
+ s_logger.info("Starting ConsoleSessionCleanupTask...");
+ Integer retentionDays =
ConsoleAccessManager.ConsoleSessionCleanupRetentionDays.value();
+ Date date = GregorianCalendar.getInstance().getTime();
+ Date dateBefore = new Date(date.getTime() - retentionDays * 24 *
3600 * 1000);
+ List<ConsoleSessionVO> sessionsToExpunge =
consoleSessionDao.listRemovedSessionsOlderThanDate(dateBefore);
+ if (CollectionUtils.isNotEmpty(sessionsToExpunge)) {
Review Comment:
I think this could be replaced by a method in ConsoleSessionDao which calls
"dao.expunge(sc)".
It can reduce the number of write operations to DB.
--
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]
