From d7080047fa7af49de27d98ec4796dfc857d297cb Mon Sep 17 00:00:00 2001 From: Leonid Andreev Date: Wed, 22 Sep 2021 16:43:18 -0400 Subject: [PATCH 01/30] A fix for one instance of lots of unnecessary queries in the header fragment. (#7804) (may still rearrange the code before finalizing) --- .../iq/dataverse/PermissionsWrapper.java | 49 +++++++++++++++++++ .../harvard/iq/dataverse/SettingsWrapper.java | 18 +++++++ src/main/webapp/dataverse_header.xhtml | 4 +- 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/PermissionsWrapper.java b/src/main/java/edu/harvard/iq/dataverse/PermissionsWrapper.java index 92892a7fe77..9e109e1aa9a 100644 --- a/src/main/java/edu/harvard/iq/dataverse/PermissionsWrapper.java +++ b/src/main/java/edu/harvard/iq/dataverse/PermissionsWrapper.java @@ -6,12 +6,14 @@ package edu.harvard.iq.dataverse; import edu.harvard.iq.dataverse.authorization.Permission; +import edu.harvard.iq.dataverse.authorization.groups.impl.builtin.AuthenticatedUsers; import edu.harvard.iq.dataverse.authorization.users.User; import edu.harvard.iq.dataverse.engine.command.Command; import edu.harvard.iq.dataverse.engine.command.DataverseRequest; import edu.harvard.iq.dataverse.engine.command.impl.*; import java.util.HashMap; import java.util.Map; +import java.util.logging.Logger; import javax.ejb.EJB; import javax.faces.view.ViewScoped; import javax.inject.Inject; @@ -24,6 +26,7 @@ @ViewScoped @Named public class PermissionsWrapper implements java.io.Serializable { + private static final Logger logger = Logger.getLogger(PermissionsWrapper.class.getName()); @EJB PermissionServiceBean permissionService; @@ -33,6 +36,9 @@ public class PermissionsWrapper implements java.io.Serializable { @Inject DataverseRequestServiceBean dvRequestService; + + @Inject + SettingsWrapper settingsWrapper; private final Map>, Boolean>> commandMap = new HashMap<>(); @@ -238,7 +244,50 @@ public boolean canIssuePublishDatasetCommand(DvObject dvo){ return canIssueCommand(dvo, PublishDatasetCommand.class); } + // For the dataverse_header fragment (and therefore, most of the pages), + // we need to know if authorized users can add dataverses and datasets to the + // root collection. + // Not a very expensive operation - but it'll add up quickly, if the + // page keeps asking for it repeatedly. So these values absolutely need to be + // cached. + Boolean showAddDataverseLink = null; + + public boolean showAddDataverseLink() { + logger.info("in showAddDataverseLink"); + if (showAddDataverseLink != null) { + logger.info("using cached showDataverseLink value"); + return showAddDataverseLink; + } + try { + showAddDataverseLink = permissionService.userOn(AuthenticatedUsers.get(), settingsWrapper.getRootDataverse()).canIssueCommand("CreateDataverseCommand"); + logger.info("rerieved showDataverseLink value"); + return showAddDataverseLink; + } catch (ClassNotFoundException ex) { + logger.info("ClassNotFoundException checking if authenticated users can create dataverses in root."); + } + + return false; + } + + Boolean showAddDatasetLink = null; + + public boolean showAddDatasetLink() { + logger.info("in showAddDatasetLink"); + if (showAddDatasetLink != null) { + logger.info("using cached showDatasetLink value"); + return showAddDatasetLink; + } + try { + showAddDatasetLink = permissionService.userOn(AuthenticatedUsers.get(), settingsWrapper.getRootDataverse()).canIssueCommand("AbstractCreateDatasetCommand"); + logger.info("rerieved showDatasetLink value"); + return showAddDatasetLink; + } catch (ClassNotFoundException ex) { + logger.info("ClassNotFoundException checking if authenticated users can create datasets in root."); + } + + return false; + } diff --git a/src/main/java/edu/harvard/iq/dataverse/SettingsWrapper.java b/src/main/java/edu/harvard/iq/dataverse/SettingsWrapper.java index 752e3e879fe..29fbe45fc22 100644 --- a/src/main/java/edu/harvard/iq/dataverse/SettingsWrapper.java +++ b/src/main/java/edu/harvard/iq/dataverse/SettingsWrapper.java @@ -57,6 +57,9 @@ public class SettingsWrapper implements java.io.Serializable { // Related to a specific setting for guide urls private String guidesBaseUrl = null; + private String siteUrl = null; + + private Dataverse rootDataverse = null; public String get(String settingKey) { if (settingsMap == null) { @@ -158,6 +161,13 @@ public String getGuidesVersion() { return systemConfig.getGuidesVersion(); } + public String getDataverseSiteUrl() { + if (siteUrl == null) { + siteUrl = systemConfig.getDataverseSiteUrl(); + } + return siteUrl; + } + public Long getZipDownloadLimit(){ return systemConfig.getZipDownloadLimit(); } @@ -372,6 +382,14 @@ public String getDefaultMetadataLanguage() { return null; } } + + Dataverse getRootDataverse() { + if (rootDataverse == null) { + rootDataverse = dataverseService.findRootDataverse(); + } + + return rootDataverse; + } } diff --git a/src/main/webapp/dataverse_header.xhtml b/src/main/webapp/dataverse_header.xhtml index 7e54b6cbec1..14e5a3f1686 100644 --- a/src/main/webapp/dataverse_header.xhtml +++ b/src/main/webapp/dataverse_header.xhtml @@ -11,8 +11,8 @@ xmlns:iqbs="http://xmlns.jcp.org/jsf/composite/iqbs"> - - + +
From 27cd64882ff31d70fbd50a6265d1fd7f83d150b9 Mon Sep 17 00:00:00 2001 From: Leonid Andreev Date: Wed, 29 Sep 2021 17:00:05 -0400 Subject: [PATCH 02/30] A flyway script that creates a custom function to replace a query that becomes unreasonably expensive for an installation with a lot of download/access activity. This solution is very efficient, but very PostgresQL-specific, hence the use of a stored function. (#7804) --- .../iq/dataverse/GuestbookResponse.java | 7 +++++ .../GuestbookResponseServiceBean.java | 30 ++++++++++++++++++- .../V5.6.0.1__7804-page-optimizations.sql | 22 ++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 src/main/resources/db/migration/V5.6.0.1__7804-page-optimizations.sql diff --git a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java index d3283fd3945..69404482fce 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java +++ b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponse.java @@ -20,6 +20,13 @@ * * @author skraffmiller */ +@NamedStoredProcedureQuery( + name = "GuestbookResponse.estimateGuestBookResponseTableSize", + procedureName = "estimateGuestBookResponseTableSize", + parameters = { + @StoredProcedureParameter(mode = ParameterMode.OUT, type = Long.class) + } +) @Entity @Table(indexes = { @Index(columnList = "guestbook_id"), diff --git a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java index 2683eaa410f..2bd4e3f3d1d 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java @@ -33,6 +33,7 @@ import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; import javax.persistence.Query; +import javax.persistence.StoredProcedureQuery; import javax.persistence.TypedQuery; /** @@ -917,7 +918,34 @@ public Long getCountGuestbookResponsesByDatasetId(Long datasetId) { } public Long getCountOfAllGuestbookResponses() { - // dataset id is null, will return 0 + // dataset id is null, will return 0 + + // "SELECT COUNT(*)" is notoriously expensive in PostgresQL for large + // tables. This makes this call fairly expensive for any installation + // with a lot of download/access activity. (This "total download" metrics + // is always displayed when the homepage is loaded). + // It is safe to say that no real life user will need this number to be + // precise down to the last few digits, and/or withing a second, + // especially once that number is in the millions. The + // solution implemented below relies on estimating the number. It is + // very efficient, but also very PostgresQL specific - hence it is + // defined as a custom database function. + // An alternative solution would be to get the exact number every + // hour or so, and cache it centrally for the rest of the application + // somehow. -- L.A. 5.6 + + + try { + StoredProcedureQuery query = this.em.createNamedStoredProcedureQuery("GuestbookResponse.estimateGuestBookResponseTableSize"); + query.execute(); + Long totalCount = (Long) query.getOutputParameterValue(1); + + if (totalCount != null) { + return totalCount; + } + } catch (IllegalArgumentException iae) { + // Don't do anything, we'll fall back to using "SELECT COUNT()" + } Query query = em.createNativeQuery("select count(o.id) from GuestbookResponse o;"); return (Long) query.getSingleResult(); } diff --git a/src/main/resources/db/migration/V5.6.0.1__7804-page-optimizations.sql b/src/main/resources/db/migration/V5.6.0.1__7804-page-optimizations.sql new file mode 100644 index 00000000000..001b6dbc68d --- /dev/null +++ b/src/main/resources/db/migration/V5.6.0.1__7804-page-optimizations.sql @@ -0,0 +1,22 @@ +-- This creates a function that ESTIMATES the size of the +-- GuestbookResponse table (for the metrics display), instead +-- of relying on straight "SELECT COUNT(*) ..." +-- Significant potential savings for an active installation. + +CREATE OR REPLACE FUNCTION estimateGuestBookResponseTableSize() +RETURNS bigint AS $$ +DECLARE + estimatedsize bigint; +BEGIN + SELECT ((reltuples / relpages) + * (pg_relation_size('public.guestbookresponse') / current_setting('block_size')::int) + )::bigint + FROM pg_class + WHERE oid = 'public.guestbookresponse'::regclass INTO estimatedsize; + + RETURN estimatedsize; +END; +$$ LANGUAGE plpgsql IMMUTABLE; + + + From 400165e117dfdd319b0f2165364efc428efe8898 Mon Sep 17 00:00:00 2001 From: Leonid Andreev Date: Wed, 29 Sep 2021 17:25:14 -0400 Subject: [PATCH 03/30] consolidatig more fixes in the same branch (#7804) --- .../edu/harvard/iq/dataverse/SettingsWrapper.java | 13 ++++++++----- src/main/webapp/dataverse_header.xhtml | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/SettingsWrapper.java b/src/main/java/edu/harvard/iq/dataverse/SettingsWrapper.java index 29fbe45fc22..d3391e3e0e6 100644 --- a/src/main/java/edu/harvard/iq/dataverse/SettingsWrapper.java +++ b/src/main/java/edu/harvard/iq/dataverse/SettingsWrapper.java @@ -60,7 +60,9 @@ public class SettingsWrapper implements java.io.Serializable { private String siteUrl = null; private Dataverse rootDataverse = null; - + + private String guidesVersion = null; + public String get(String settingKey) { if (settingsMap == null) { initSettingsMap(); @@ -137,9 +139,7 @@ private void initSettingsMap() { public String getGuidesBaseUrl() { - if (true) - - if (guidesBaseUrl == null) { + if (guidesBaseUrl == null) { String saneDefault = "https://guides.dataverse.org"; guidesBaseUrl = getValueForKey(SettingsServiceBean.Key.GuidesBaseUrl); @@ -158,7 +158,10 @@ public String getGuidesBaseUrl() { } public String getGuidesVersion() { - return systemConfig.getGuidesVersion(); + if (guidesVersion == null) { + guidesVersion = systemConfig.getGuidesVersion(); + } + return guidesVersion; } public String getDataverseSiteUrl() { diff --git a/src/main/webapp/dataverse_header.xhtml b/src/main/webapp/dataverse_header.xhtml index 14e5a3f1686..40678c6dc67 100644 --- a/src/main/webapp/dataverse_header.xhtml +++ b/src/main/webapp/dataverse_header.xhtml @@ -73,7 +73,7 @@
  • - +
    • From 78c2d5738dc83268d6110bf731df1b9b2aee61eb Mon Sep 17 00:00:00 2001 From: Leonid Andreev Date: Wed, 29 Sep 2021 22:27:22 -0400 Subject: [PATCH 04/30] multiple lookups of the root dataverse from the header fragment (#7804) --- .../java/edu/harvard/iq/dataverse/SettingsWrapper.java | 2 +- src/main/webapp/dataverse_header.xhtml | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/SettingsWrapper.java b/src/main/java/edu/harvard/iq/dataverse/SettingsWrapper.java index d3391e3e0e6..876ca9c4b21 100644 --- a/src/main/java/edu/harvard/iq/dataverse/SettingsWrapper.java +++ b/src/main/java/edu/harvard/iq/dataverse/SettingsWrapper.java @@ -386,7 +386,7 @@ public String getDefaultMetadataLanguage() { } } - Dataverse getRootDataverse() { + public Dataverse getRootDataverse() { if (rootDataverse == null) { rootDataverse = dataverseService.findRootDataverse(); } diff --git a/src/main/webapp/dataverse_header.xhtml b/src/main/webapp/dataverse_header.xhtml index 40678c6dc67..11b9e28bd58 100644 --- a/src/main/webapp/dataverse_header.xhtml +++ b/src/main/webapp/dataverse_header.xhtml @@ -31,7 +31,7 @@ #{bundle.dataverse} + url="#{settingsWrapper.get(':LogoCustomizationFile')}" styleClass="navbar-brand custom-logo" alt="#{of:format1(bundle['alt.homepage'], settingsWrapper.rootDataverse.name)}"/>