Details

    • Branch Version/s:
      6.1.x, 6.0.x
    • Backported to Branch:
      Committed
    • Similar Issues:
      Show 4 results 

      Description

      When I have about 10 000 AssetEntry and one asset publisher in my page and I want display all of them - Asset Publisher sends 30 000 queries to database in one request.
      I've written about it here: http://liferaycms.wordpress.com/2011/10/11/one-of-bottleneck-in-liferay-6-1/ and here: http://liferaycms.wordpress.com/2011/10/17/asset-publisher-and-queries-count/

        Issue Links

          Activity

          Hide
          Piotr Filipowicz added a comment -

          Guys, I thought on this topic a lot. Maybe we should modify PermissionChecker implementation. My inspiration was Lucene index. IMHO we can do that by adding new table AssetEntry_ViewPermission with 4 columns:

          • id_
          • entryId
          • groupId
          • roleId
            If we want to add groupRoleId we put groupId and roleId values otherwise if we want to add roleId - the value of groupId will be zero.
            Thus we can check View permission using SQL query by join our new table (lucene indexer do that thing).

          Advantages:

          • we will have only one query to dabase in one Asset
          • we won't get 5000 records just in case
          • we won't get specific entry (e.g. JournalArticle, Blog and so on) if we want display Asset's abstract view.

          Second way is a full PermissionChecker refactoring and link it with AssetEntries.

          Third way is to change AssetPublisher implementation and use LuceneIndexer to pick records. We'll try to test this solution and measure the performance.

          Show
          Piotr Filipowicz added a comment - Guys, I thought on this topic a lot. Maybe we should modify PermissionChecker implementation. My inspiration was Lucene index. IMHO we can do that by adding new table AssetEntry_ViewPermission with 4 columns: id_ entryId groupId roleId If we want to add groupRoleId we put groupId and roleId values otherwise if we want to add roleId - the value of groupId will be zero. Thus we can check View permission using SQL query by join our new table (lucene indexer do that thing). Advantages: we will have only one query to dabase in one Asset we won't get 5000 records just in case we won't get specific entry (e.g. JournalArticle, Blog and so on) if we want display Asset's abstract view. Second way is a full PermissionChecker refactoring and link it with AssetEntries. Third way is to change AssetPublisher implementation and use LuceneIndexer to pick records. We'll try to test this solution and measure the performance.
          Hide
          Raymond Auge added a comment -

          I would recommend the last suggestion of simply re-implementing AP to use the index from the beginning!

          Show
          Raymond Auge added a comment - I would recommend the last suggestion of simply re-implementing AP to use the index from the beginning!
          Hide
          Jorge Ferrer added a comment -

          I agree that using the index could be a solution medium-term.

          But short-term I think there should be easier solutions. Also I'm not sure if the index could replace all the functionalities of the db so fixing that is important.

          The very basic solution is to make it so that by default it is not guaranteed that the exact number of items configured are retrieved (if permissions apply to hide some of them). IMHO, for a very large percentage of usage of AP, that will be just fine, specially for those that are used in public pages where most content is public. Those are also the cases that will care about performance first.

          We could then add a checkbox next to the selection of the number of items to enable "Exact count of items" which has a help icon that explains that this may have performance implications. In any case I would also look for opportunities for improvement in the current algorithm. It may not be optimal in most cases to grab 5000 records ahead of time. A 3-10 times the number selected (or something in that order of magnitude) is probably more reasonable.

          Thoughts?

          Show
          Jorge Ferrer added a comment - I agree that using the index could be a solution medium-term. But short-term I think there should be easier solutions. Also I'm not sure if the index could replace all the functionalities of the db so fixing that is important. The very basic solution is to make it so that by default it is not guaranteed that the exact number of items configured are retrieved (if permissions apply to hide some of them). IMHO, for a very large percentage of usage of AP, that will be just fine, specially for those that are used in public pages where most content is public. Those are also the cases that will care about performance first. We could then add a checkbox next to the selection of the number of items to enable "Exact count of items" which has a help icon that explains that this may have performance implications. In any case I would also look for opportunities for improvement in the current algorithm. It may not be optimal in most cases to grab 5000 records ahead of time. A 3-10 times the number selected (or something in that order of magnitude) is probably more reasonable. Thoughts?
          Hide
          Szymon Golebiewski added a comment -

          Jorge,

          Such resolution even as a workaround is a bad idea. If we want to show 20 articles I want to see 20 articles unless I know that there are no so many in the CMS system. We can not guarantee users that "not exact count of items" won't return ZERO articles. Everty time they will see other number of articles will be counted as a bug so no will use it. Also I don't know any CMS system that has functionality that will return non-deterministic number of articles. Such poor workaround can only lead to more "Liferay hate" and any explanation in help icon will be treated as an excuse.

          Show
          Szymon Golebiewski added a comment - Jorge, Such resolution even as a workaround is a bad idea. If we want to show 20 articles I want to see 20 articles unless I know that there are no so many in the CMS system. We can not guarantee users that "not exact count of items" won't return ZERO articles. Everty time they will see other number of articles will be counted as a bug so no will use it. Also I don't know any CMS system that has functionality that will return non-deterministic number of articles. Such poor workaround can only lead to more "Liferay hate" and any explanation in help icon will be treated as an excuse.
          Hide
          Jorge Ferrer added a comment -

          Hi Szymon,

          I understand what you mean. I still think that we need to find the best possible compromise solution.

          In 6.0 and previous asset publisher didn't do any permission check at all, so Ray added that capability for 6.1 which is what has caused the potential scalability problem. We have already made several improvements including making sure that the portlet makes the minimum number of db requests (previously, as pointed out in Piotr's blog it was making several unnecessary requests).

          I also think that the default filter limit should be lowered from 5000 to something around a few hundreths (or maybe a multiplier of the number of items configured by that portlet). What do you guys think about this?

          Finally, I think that we should provide an option to disable permission checks until we find a long term solution that allows for scalable permission check queries. I thought that it was better to still do a permission check on the frontend even if that implied that the count was inaccurate (or even the list ends up being empty), but an alternative is to avoid it altogether.

          Jorge

          Show
          Jorge Ferrer added a comment - Hi Szymon, I understand what you mean. I still think that we need to find the best possible compromise solution. In 6.0 and previous asset publisher didn't do any permission check at all, so Ray added that capability for 6.1 which is what has caused the potential scalability problem. We have already made several improvements including making sure that the portlet makes the minimum number of db requests (previously, as pointed out in Piotr's blog it was making several unnecessary requests). I also think that the default filter limit should be lowered from 5000 to something around a few hundreths (or maybe a multiplier of the number of items configured by that portlet). What do you guys think about this? Finally, I think that we should provide an option to disable permission checks until we find a long term solution that allows for scalable permission check queries. I thought that it was better to still do a permission check on the frontend even if that implied that the count was inaccurate (or even the list ends up being empty), but an alternative is to avoid it altogether. Jorge
          Hide
          Szymon Golebiewski added a comment -

          Hi Jorge,

          Solution based on limiting number of default filter will be always slower than lucene. Also the lower limit we make the higher goes possibility of wrong results. Also disabling permissions checks is not the way our customers will accept.

          So having all that in mind we prepared solution based on Lucene. You can read about our results on our blog our blog. We think that's the best option for both short and long term solution.

          Show
          Szymon Golebiewski added a comment - Hi Jorge, Solution based on limiting number of default filter will be always slower than lucene. Also the lower limit we make the higher goes possibility of wrong results. Also disabling permissions checks is not the way our customers will accept. So having all that in mind we prepared solution based on Lucene. You can read about our results on our blog our blog . We think that's the best option for both short and long term solution.
          Hide
          Szymon Golebiewski added a comment -

          Any news about that thing?

          Show
          Szymon Golebiewski added a comment - Any news about that thing?
          Hide
          Piotr Filipowicz added a comment -

          Michael, is your solution rely on enable/disable checkin' permissions? I saw your commit. Did you add enablePermissions flag?

          Show
          Piotr Filipowicz added a comment - Michael, is your solution rely on enable/disable checkin' permissions? I saw your commit. Did you add enablePermissions flag?
          Hide
          Michael Han added a comment -

          For short term:
          (1) Provide ability to enable/disable permission checks
          (2) Reduce amount of permission checks executed and added more caching

          Show
          Michael Han added a comment - For short term: (1) Provide ability to enable/disable permission checks (2) Reduce amount of permission checks executed and added more caching
          Hide
          Piotr Filipowicz added a comment -

          (2) Reduce amount of permission checks you mean remove 'assetRenderer.hasViewPermission(permissionChecker)' on asset's views (jsps)?

          Show
          Piotr Filipowicz added a comment - (2) Reduce amount of permission checks you mean remove 'assetRenderer.hasViewPermission(permissionChecker)' on asset's views (jsps)?
          Hide
          Michael Han added a comment -

          @Piotr, no there were some situations where the same query was happening multiple times so we activated more caching to avoid this.

          Show
          Michael Han added a comment - @Piotr, no there were some situations where the same query was happening multiple times so we activated more caching to avoid this.
          Hide
          Juan G added a comment -

          Seems this issue caused this one: LPS-26062

          Show
          Juan G added a comment - Seems this issue caused this one: LPS-26062
          Hide
          Matthew Lee (Inactive) added a comment -

          Committed on:
          Portal 6.1.x CE GIT ID: 7e448632ac9f81d5a4bd1b088016fa1ceb95c990.
          Portal 6.2.x GIT ID: 7e448632ac9f81d5a4bd1b088016fa1ceb95c990.

          Show
          Matthew Lee (Inactive) added a comment - Committed on: Portal 6.1.x CE GIT ID: 7e448632ac9f81d5a4bd1b088016fa1ceb95c990. Portal 6.2.x GIT ID: 7e448632ac9f81d5a4bd1b088016fa1ceb95c990.

            People

            • Votes:
              4 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Days since last comment:
                2 years, 39 weeks, 5 days ago

                Development

                  Structure Helper Panel