From 79337f60aaa66787ce0b6eec0ff5b318c70f8700 Mon Sep 17 00:00:00 2001 From: eea03 Date: Thu, 5 Nov 2015 17:00:30 +0100 Subject: [PATCH 1/2] Introduced caches for pipelines and executions in corresponding views to improve performance. Until now, unreasonable count of selects were pointlessly generated every time table was refreshed, even though data were not changed and there was no point to reload them --- .../ExecutionListPresenterImpl.java | 67 ++++++++++++++----- .../PipelineListPresenterImpl.java | 51 +++++++++++++- 2 files changed, 99 insertions(+), 19 deletions(-) diff --git a/frontend/src/main/java/cz/cuni/mff/xrg/odcs/frontend/gui/views/executionlist/ExecutionListPresenterImpl.java b/frontend/src/main/java/cz/cuni/mff/xrg/odcs/frontend/gui/views/executionlist/ExecutionListPresenterImpl.java index d43ab51229..cf0143ec80 100644 --- a/frontend/src/main/java/cz/cuni/mff/xrg/odcs/frontend/gui/views/executionlist/ExecutionListPresenterImpl.java +++ b/frontend/src/main/java/cz/cuni/mff/xrg/odcs/frontend/gui/views/executionlist/ExecutionListPresenterImpl.java @@ -17,6 +17,7 @@ package cz.cuni.mff.xrg.odcs.frontend.gui.views.executionlist; import java.util.Date; +import java.util.HashMap; import java.util.List; import java.util.Map; @@ -38,7 +39,6 @@ import cz.cuni.mff.xrg.odcs.commons.app.execution.message.DbMessageRecord; import cz.cuni.mff.xrg.odcs.commons.app.execution.message.MessageRecord; import cz.cuni.mff.xrg.odcs.commons.app.facade.PipelineFacade; -import cz.cuni.mff.xrg.odcs.commons.app.pipeline.DbExecution; import cz.cuni.mff.xrg.odcs.commons.app.pipeline.PipelineExecution; import cz.cuni.mff.xrg.odcs.commons.app.pipeline.PipelineExecutionStatus; import cz.cuni.mff.xrg.odcs.frontend.AppEntry; @@ -70,9 +70,6 @@ public class ExecutionListPresenterImpl implements ExecutionListPresenter, PostL private static final Logger LOG = LoggerFactory.getLogger(ExecutionListPresenterImpl.class); - @Autowired - private DbExecution dbExecution; - @Autowired private DBExecutionView dbExecutionView; @@ -106,6 +103,14 @@ public class ExecutionListPresenterImpl implements ExecutionListPresenter, PostL private boolean isInitialized = false; + private Map lightExecutionCache = new HashMap<>(); + + private static final int LIGHT_CACHE_MAX_SIZE = 100; + + private static final int LIGHT_CACHE_TIMEOUT = 300000; + + private Date lightCacheLastReload = new Date(); + @Override public Object enter() { if (isInitialized) { @@ -198,10 +203,18 @@ public void setParameters(Object configuration) { @Override public void refreshEventHandler() { - boolean hasModifiedExecutions = pipelineFacade.hasModifiedExecutions(lastLoad) + boolean hasModifiedExecutions = this.pipelineFacade.hasModifiedExecutions(this.lastLoad) || (cachedSource.size() > 0 && - pipelineFacade.hasDeletedExecutions((List) cachedSource.getItemIds(0, cachedSource.size()))); - view.refresh(hasModifiedExecutions); + pipelineFacade.hasDeletedExecutions((List) cachedSource.getItemIds(0, cachedSource.size()))); + boolean hasModifiedPipelines = this.pipelineFacade.hasModifiedPipelines(this.lastLoad); + + // reload light cache only if some pipeline has changed as this can cause change of access rights for execution + // as they are based on pipeline access rights + if (hasModifiedPipelines) { + reloadLightExecutionCache(); + } + + this.view.refresh(hasModifiedExecutions); if (hasModifiedExecutions) { lastLoad = new Date(); cachedSource.invalidate(); @@ -211,8 +224,7 @@ public void refreshEventHandler() { @Override public boolean canStopExecution(long executionId) { - PipelineExecution exec = - getLightExecution(executionId); + PipelineExecution exec = getLightExecution(executionId); return permissionUtils.hasPermission(exec, EntityPermissions.PIPELINE_EXECUTION_STOP); } @@ -253,12 +265,35 @@ public void debugEventHandler(long executionId) { /** * Get light copy of execution. + * Light executions are cached as this method is called multiple times per one execution * * @param executionId * @return light copy of execution */ private PipelineExecution getLightExecution(long executionId) { - return pipelineFacade.getExecution(executionId); + if (this.lightExecutionCache.size() >= LIGHT_CACHE_MAX_SIZE) { + reloadLightExecutionCache(); + } + + if (this.lightCacheLastReload.before(new Date(new Date().getTime() - LIGHT_CACHE_TIMEOUT))) { + LOG.debug("Light execution cache timeout, reloading ..."); + reloadLightExecutionCache(); + } + + PipelineExecution exec = null; + if (this.lightExecutionCache.containsKey(executionId)) { + exec = this.lightExecutionCache.get(executionId); + } else { + exec = this.pipelineFacade.getExecution(executionId); + this.lightExecutionCache.put(executionId, exec); + } + + return exec; + } + + private void reloadLightExecutionCache() { + this.lightExecutionCache.clear(); + this.lightCacheLastReload = new Date(); } private ReadOnlyContainer getMessageDataSource() { @@ -332,29 +367,25 @@ public boolean isLayoutInitialized() { @Override public boolean canReadLog(long executionId) { - PipelineExecution exec = - getLightExecution(executionId); + PipelineExecution exec = getLightExecution(executionId); return permissionUtils.hasPermission(exec, EntityPermissions.PIPELINE_EXECUTION_READ); } @Override public boolean canDebugData(long executionId) { - PipelineExecution exec = - getLightExecution(executionId); + PipelineExecution exec = getLightExecution(executionId); return permissionUtils.hasPermission(exec, EntityPermissions.PIPELINE_EXECUTION_READ); } @Override public boolean canRunPipeline(long executionId) { - PipelineExecution exec = - getLightExecution(executionId); + PipelineExecution exec = getLightExecution(executionId); return permissionUtils.hasPermission(exec, EntityPermissions.PIPELINE_RUN); } @Override public boolean canDebugPipeline(long executionId) { - PipelineExecution exec = - getLightExecution(executionId); + PipelineExecution exec = getLightExecution(executionId); return this.permissionUtils.hasUserAuthority(EntityPermissions.PIPELINE_RUN_DEBUG) && this.permissionUtils.hasPermission(exec, EntityPermissions.PIPELINE_RUN_DEBUG); } diff --git a/frontend/src/main/java/cz/cuni/mff/xrg/odcs/frontend/gui/views/pipelinelist/PipelineListPresenterImpl.java b/frontend/src/main/java/cz/cuni/mff/xrg/odcs/frontend/gui/views/pipelinelist/PipelineListPresenterImpl.java index 5f594dade1..d4cf4cf73f 100644 --- a/frontend/src/main/java/cz/cuni/mff/xrg/odcs/frontend/gui/views/pipelinelist/PipelineListPresenterImpl.java +++ b/frontend/src/main/java/cz/cuni/mff/xrg/odcs/frontend/gui/views/pipelinelist/PipelineListPresenterImpl.java @@ -17,6 +17,7 @@ package cz.cuni.mff.xrg.odcs.frontend.gui.views.pipelinelist; import java.util.Date; +import java.util.HashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -116,6 +117,17 @@ public class PipelineListPresenterImpl implements PipelineListPresenter, PostLog private Date lastLoad = new Date(0L); + /** + * Pipeline cache for light pipelines loading + */ + private Map lightPipelineCache = new HashMap<>(); + + private static final int LIGHT_CACHE_MAX_SIZE = 100; + + private static final int LIGHT_CACHE_TIMEOUT = 300000; + + private Date lightCacheLastReload = new Date(); + /** * Application's configuration. */ @@ -177,6 +189,11 @@ public void refresh(Refresher source) { || hasDeletedExecutions; LOG.debug("Last load: {}, hasModifiedPipelines: {}, hasModifiedExecutions: {}, hasDeletedPipelines: {}", lastLoad, hasModifiedPipelines, hasModifiedExecutions, hasDeletedExecutions); + + if (hasModifiedPipelines) { + reloadLightPipelineCache(); + } + if (hasModifiedPipelinesOrExecutions) { LOG.debug("Execution / pipeline modified, refreshing ..."); lastLoad = new Date(); @@ -186,6 +203,7 @@ public void refresh(Refresher source) { lastRefreshFinished = new Date().getTime(); } } + }); this.refreshManager.triggerRefresh(); } @@ -355,8 +373,39 @@ public void navigateToEventHandler(Class where, Object param) { } } + /** + * Get light copy of pipeline. + * Light executions are cached as this method is called multiple times per one pipeline. + * + * @param pipelineId + * @return light copy of pipeline + */ private Pipeline getLightPipeline(long pipelineId) { - return pipelineFacade.getPipeline(pipelineId); + if (this.lightPipelineCache.size() >= LIGHT_CACHE_MAX_SIZE) { + LOG.debug("Light pipeline cache size exceeded, reloading ..."); + reloadLightPipelineCache(); + } + + if (this.lightCacheLastReload.before(new Date(new Date().getTime() - LIGHT_CACHE_TIMEOUT))) { + LOG.debug("Light pipeline cache timeout, reloading ..."); + reloadLightPipelineCache(); + } + + Pipeline pipeline = null; + if (this.lightPipelineCache.containsKey(pipelineId)) { + pipeline = this.lightPipelineCache.get(pipelineId); + } else { + pipeline = this.pipelineFacade.getPipeline(pipelineId); + this.lightPipelineCache.put(pipelineId, pipeline); + } + + return pipeline; + + } + + private void reloadLightPipelineCache() { + this.lightPipelineCache.clear(); + this.lightCacheLastReload = new Date(); } @Override From dbd6fe2bf077984753b4ae96abaf88b4c915f401 Mon Sep 17 00:00:00 2001 From: eea03 Date: Mon, 9 Nov 2015 14:56:56 +0100 Subject: [PATCH 2/2] Introduced schedule cache to prevent absolutely unnecessary inidividual selects to retrieve Schedules. Fixed bug with refresher (table wasn't refreshing after schedule detail window opened and closed) --- .../odcs/frontend/gui/views/Scheduler.java | 163 ++++++++++++------ 1 file changed, 107 insertions(+), 56 deletions(-) diff --git a/frontend/src/main/java/cz/cuni/mff/xrg/odcs/frontend/gui/views/Scheduler.java b/frontend/src/main/java/cz/cuni/mff/xrg/odcs/frontend/gui/views/Scheduler.java index 14963b4743..c3e2f5fd32 100644 --- a/frontend/src/main/java/cz/cuni/mff/xrg/odcs/frontend/gui/views/Scheduler.java +++ b/frontend/src/main/java/cz/cuni/mff/xrg/odcs/frontend/gui/views/Scheduler.java @@ -18,6 +18,7 @@ import java.text.DateFormat; import java.util.Date; +import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -32,8 +33,6 @@ import org.tepi.filtertable.paged.PagedTableChangeEvent; import org.vaadin.dialogs.ConfirmDialog; -import ru.xpoft.vaadin.VaadinView; - import com.github.wolfie.refresher.Refresher; import com.vaadin.data.Property; import com.vaadin.data.util.IndexedContainer; @@ -42,9 +41,15 @@ import com.vaadin.server.Page; import com.vaadin.server.Resource; import com.vaadin.server.ThemeResource; -import com.vaadin.ui.*; +import com.vaadin.ui.Button; import com.vaadin.ui.Button.ClickEvent; import com.vaadin.ui.Button.ClickListener; +import com.vaadin.ui.CustomTable; +import com.vaadin.ui.Embedded; +import com.vaadin.ui.HorizontalLayout; +import com.vaadin.ui.Notification; +import com.vaadin.ui.UI; +import com.vaadin.ui.VerticalLayout; import com.vaadin.ui.Window.CloseEvent; import com.vaadin.ui.Window.CloseListener; @@ -68,6 +73,7 @@ import cz.cuni.mff.xrg.odcs.frontend.i18n.Messages; import cz.cuni.mff.xrg.odcs.frontend.navigation.Address; import cz.cuni.mff.xrg.odcs.frontend.navigation.ParametersHandler; +import ru.xpoft.vaadin.VaadinView; /** * GUI for Scheduler page which opens from the main menu. Contains table with @@ -147,6 +153,17 @@ public class Scheduler extends ViewComponent implements PostLogoutCleaner, Prese private boolean isMainLayoutInitialized = false; + /** + * Pipeline cache for light pipelines loading + */ + private Map scheduleCache = new HashMap<>(); + + private static final int CACHE_MAX_SIZE = 100; + + private static final int CACHE_TIMEOUT = 300000; + + private Date lightCacheLastReload = new Date(); + /** * The constructor should first build the main layout, set the composition * root and then do any custom initialization. @@ -170,8 +187,27 @@ public void enter(ViewChangeEvent event) { } setCompositionRoot(mainLayout); - refreshManager = ((AppEntry) UI.getCurrent()).getRefreshManager(); - refreshManager.addListener(RefreshManager.SCHEDULER, new Refresher.RefreshListener() { + addRefreshListener(); + + setParameters(ParametersHandler.getConfiguration(event.getParameters())); + } + + @Override + public Object enter() { + if (!isMainLayoutInitialized) { + buildMainLayout(); + isMainLayoutInitialized = true; + } + setCompositionRoot(mainLayout); + + addRefreshListener(); + + return this; + } + + private void addRefreshListener() { + this.refreshManager = ((AppEntry) UI.getCurrent()).getRefreshManager(); + this.refreshManager.addListener(RefreshManager.SCHEDULER, new Refresher.RefreshListener() { private long lastRefreshFinished = 0; @Override @@ -179,16 +215,17 @@ public void refresh(Refresher source) { if (new Date().getTime() - lastRefreshFinished > RefreshManager.MIN_REFRESH_INTERVAL) { boolean hasModifiedExecutions = pipelineFacade.hasModifiedExecutions(lastLoad); if (hasModifiedExecutions) { + LOG.debug("Modified executions found, refreshing ..."); lastLoad = new Date(); refreshData(); + LOG.debug("Scheduler refreshed."); } - LOG.debug("Scheduler refreshed."); + lastRefreshFinished = new Date().getTime(); } } }); - refreshManager.triggerRefresh(); - setParameters(ParametersHandler.getConfiguration(event.getParameters())); + this.refreshManager.triggerRefresh(); } /** @@ -460,15 +497,17 @@ private static String getScheduledByDisplayName(Schedule schedule) { /** * Calls for refresh table {@link #schedulerTable}. */ + @SuppressWarnings("deprecation") private void refreshData() { + List schedules = this.scheduleFacade.getAllSchedules(); + reloadScheduleCache(schedules); int page = schedulerTable.getCurrentPage(); - tableData = getTableData(scheduleFacade.getAllSchedules()); + tableData = getTableData(schedules); schedulerTable.setContainerDataSource(tableData); schedulerTable.setCurrentPage(page); schedulerTable.setVisibleColumns((Object[]) visibleCols); schedulerTable.setFilterFieldVisible("commands", false); schedulerTable.setFilterFieldVisible("duration", false); - } /** @@ -486,13 +525,14 @@ private void showSchedulePipeline(Long id, Long pipelineId) { @Override public void windowClose(CloseEvent e) { refreshData(); + addRefreshListener(); } }); } Schedule schedule = null; if (id != null) { - schedule = scheduleFacade.getSchedule(id); + schedule = getSchedule(id); } schedulePipeline.setSelectedSchedule(schedule); schedulePipeline.enableComboPipeline(); @@ -520,7 +560,7 @@ class actionColumnGenerator implements CustomTable.ColumnGenerator { @Override public Object generateCell(final CustomTable source, final Object itemId, Object columnId) { final Long schId = Long.parseLong(tableData.getContainerProperty(itemId, "schid").getValue().toString()); - Schedule schedule = scheduleFacade.getSchedule(schId); + Schedule schedule = getSchedule(schId); Property propStatus = source.getItem(itemId).getItemProperty("status"); HorizontalLayout layout = new HorizontalLayout(); @@ -589,22 +629,22 @@ public void buttonClick(ClickEvent event) { deleteButton.addClickListener(new ClickListener() { @Override public void buttonClick(ClickEvent event) { - scheduleDel = scheduleFacade.getSchedule(schId); + scheduleDel = getSchedule(schId); //open confirmation dialog ConfirmDialog.show(UI.getCurrent(), Messages.getString("Scheduler.delete.scheduling"), Messages.getString("Scheduler.delete.scheduling.description", scheduleDel.getPipeline().getName().toString()), Messages.getString("Scheduler.delete.scheduling.deleteButton"), Messages.getString("Scheduler.delete.scheduling.calcelButton"), new ConfirmDialog.Listener() { - private static final long serialVersionUID = 1L; - - @Override - public void onClose(ConfirmDialog cd) { - if (cd.isConfirmed()) { - scheduleFacade.delete(scheduleDel); - refreshData(); - } - } - }); + private static final long serialVersionUID = 1L; + + @Override + public void onClose(ConfirmDialog cd) { + if (cd.isConfirmed()) { + scheduleFacade.delete(scheduleDel); + refreshData(); + } + } + }); } }); if (canDelete(schedule)) { @@ -616,9 +656,9 @@ public void onClose(ConfirmDialog cd) { } private void setScheduleEnabled(Long schId, boolean enabled) { - Schedule schedule = scheduleFacade.getSchedule(schId); + Schedule schedule = getSchedule(schId); schedule.setEnabled(enabled); - scheduleFacade.save(schedule); + this.scheduleFacade.save(schedule); } boolean canDelete(Schedule schedule) { @@ -663,36 +703,6 @@ private void changeURI(Long scheduleId) { ((AppEntry) UI.getCurrent()).setUriFragment(handler.getUriFragment(), false); } - @Override - public Object enter() { - if (!isMainLayoutInitialized) { - buildMainLayout(); - isMainLayoutInitialized = true; - } - setCompositionRoot(mainLayout); - - refreshManager = ((AppEntry) UI.getCurrent()).getRefreshManager(); - refreshManager.addListener(RefreshManager.SCHEDULER, new Refresher.RefreshListener() { - private long lastRefreshFinished = 0; - - @Override - public void refresh(Refresher source) { - if (new Date().getTime() - lastRefreshFinished > RefreshManager.MIN_REFRESH_INTERVAL) { - boolean hasModifiedExecutions = pipelineFacade.hasModifiedExecutions(lastLoad); - if (hasModifiedExecutions) { - lastLoad = new Date(); - refreshData(); - } - LOG.debug("Scheduler refreshed."); - lastRefreshFinished = new Date().getTime(); - } - } - }); - refreshManager.triggerRefresh(); - - return this; - } - @Override public void setParameters(Object configuration) { if (configuration != null && Map.class.isAssignableFrom(configuration.getClass())) { @@ -767,7 +777,7 @@ public void showDebugEventHandler(long scheduleId) { if (!schedulerTable.getItemIds().contains((new Long(scheduleId)).intValue())) { return; } - Schedule schedule = scheduleFacade.getSchedule(scheduleId); + Schedule schedule = getSchedule(scheduleId); if (schedule == null) { Notification.show(Messages.getString("Scheduler.0", scheduleId), Notification.Type.ERROR_MESSAGE); return; @@ -782,4 +792,45 @@ public void pageChangedHandler(Integer newPageNumber) { ((AppEntry) UI.getCurrent()).setUriFragment(handler.getUriFragment(), false); } + /** + * Get cached schedule or retrieve from database + * + * @param scheduleId + * @return copy of schedule + */ + private Schedule getSchedule(long scheduleId) { + if (this.scheduleCache.size() >= CACHE_MAX_SIZE) { + LOG.debug("Light pipeline cache size exceeded, reloading ..."); + reloadScheduleCache(); + } + + if (this.lightCacheLastReload.before(new Date(new Date().getTime() - CACHE_TIMEOUT))) { + LOG.debug("Light pipeline cache timeout, reloading ..."); + reloadScheduleCache(); + } + + Schedule schedule = null; + if (this.scheduleCache.containsKey(scheduleId)) { + schedule = this.scheduleCache.get(scheduleId); + } else { + schedule = this.scheduleFacade.getSchedule(scheduleId); + this.scheduleCache.put(scheduleId, schedule); + } + + return schedule; + } + + private void reloadScheduleCache() { + this.scheduleCache.clear(); + this.lightCacheLastReload = new Date(); + } + + private void reloadScheduleCache(List schedules) { + this.scheduleCache.clear(); + for (Schedule schedule : schedules) { + scheduleCache.put(schedule.getId(), schedule); + } + this.lightCacheLastReload = new Date(); + } + }