1. 14 Sep, 2022 2 commits
    • Jérome Perrin's avatar
    • Vincent Pelletier's avatar
      Products.CMFActivity.Activity.SQLBase: Drop "FOR UPDATE". · 89aa2a6b
      Vincent Pelletier authored
      This SELECT option was added when the UPDATE query was split into a
      SELECT + UPDATE query pair, in order to try to avoid deadlocks in mariadb.
        commit 7daaf0a5
        Author: Vincent Pelletier <vincent@nexedi.com>
        Date:   Fri Jun 7 16:53:11 2013 +0200
      
            Work around poor UPDATE use of index.
      That was not completely successful, and another attempt was since made,
      which further ipmproved the situation:
        commit e21aa47f601a98f767e2d3e8490b20b7f409933c
        Author: Vincent Pelletier <vincent@nexedi.com>
        Date:   Fri Sep 17 11:34:46 2021 +0900
      
            CMFActivity.Activity.SQLBase: Reduce the number of deadlocks
      
      This change made the "FOR UPDATE" option obsolete: there should be
      virtually no parallelism between the SELECT and UPDATE queries.
      By which I mean only 3rd-party accesses (like an admin on a mysql console)
      would bypass the SQL lock, causing index-update-level locking issues.
      
      Checking now, I see that removing "FOR UPDATE" on the "node>0" query makes
      that query (with LIMIT 30) run about 5 times faster:
      With FOR UPDATE:    0.509s
      Without FOR UPDATE: 0.090s
      This difference is enough to cause significant activity starvation on a
      63-processing-nodes cluster where such activities run in about 1s, just
      because of the amount of contention on the SQL lock, itself because of the
      duration of every execution of this query.
      
      So remove this now-redundant option.
      
      It is expected that this will improve activity processing performance in
      general, beyond just the starvation issue which led to this discovery.
      89aa2a6b
  2. 09 Sep, 2022 2 commits
  3. 07 Sep, 2022 4 commits
  4. 06 Sep, 2022 3 commits
    • Kazuhiko Shiozaki's avatar
      Folder: fix objectValues(portal_type=...). · 48cfa07a
      Kazuhiko Shiozaki authored
      For example, Web Section can contain a Script (Python) to set Access
      Rule, but the parent section's objectValues(portal_type='Web Section')
      should not include such object.
      48cfa07a
    • Jérome Perrin's avatar
    • Vincent Pelletier's avatar
      Products.CMFActivity: Fix poor performance with many family-bound activities · 62af8254
      Vincent Pelletier authored
      When there are many simultaneously-pending activities attached to any
      processing node family, the
        node>=0
      subquery becomes dominant (taking hundreds of time longer than the other
      subqueries). As a consequence, this starves processing nodes of activities
      and increases the CPU needs of the mariadb process hosting the activity
      tables.
      So, move this subquery out of the regular codepath, and only run it if no
      other subquery found any activity:
      - there is no activity preferentially targeting the current node
      - there is no activity bound to any of the current node's families
      - there is no activity without any node preference at all
      Also, simplify the content of that subquery: the effective priority can
      only be 3 * priority + 1 when this query is run, and node=0 rows can be
      excluded (they should not exist in the current database view).
      Also, factorise the logic producing "node=processing_node" and
      "node IN node_set" subqueries, for simplicity. In turn, this makes all
      family-dependent subqueries use a simple equality test, ensuring a stable
      query plan independently from the number of families the current node is
      member of.
      Also, use "UNION ALL" always, as now:
      - all subqueries have stritly distinct result sets
      - as per mariadb documentation, "UNION [DISTINCT] applies to all UNIONs
        on the left", so the original comment about where ALL is used was
        incorrect in assuming it was improving the effective query performance
      Also, line-split SQL queries as visible in the python source to be more
      readable, without effect on the produced SQL.
      Also, line-split a few non-trivial python expression to make their
      internal structure immediately apparent.
      
      Another effect of this change this change is to reduce activity theft
      (activities to be preferentially executed by one node being executed by
      another), potentially improving object cache hit-rate and hence decreasing
      I/O pressure on the ZODB.
      62af8254
  5. 30 Aug, 2022 3 commits
  6. 29 Aug, 2022 2 commits
    • Jérome Perrin's avatar
      zope4: TODO · 87bfa0c2
      Jérome Perrin authored
      87bfa0c2
    • Jérome Perrin's avatar
      patches: add a compatibility patch for `zope_quick_start` · 4c8d9ea8
      Jérome Perrin authored
      `zope_quick_start` was a dtml that was removed in Zope commit c8e4cd5ca
      (Simplify control panel objects and actually make them non-persistent.,
      2016-08-14), but it is referenced in /index_html page template, which
      contains:
      
          <tal:text tal:replace="structure context/zope_quick_start" />
      
      As a result, rendering / is an error page because of KeyError: 'zope_quick_start'
      
      In SlapOS setup, we are using / in an haproxy health check, so the
      health check fail and zope is marked as down and haproxy does not send
      requests to backend.
      
      We could have an upgrader constraint to repair this page template, but
      because of this, zope is unreachable through haproxy, so it's easier to
      ressort to monkey patching.
      4c8d9ea8
  7. 26 Aug, 2022 3 commits
    • Tatuya Kamada's avatar
      ERP5Catalog: Fix UnicodeDecodeError on non ascii catalog search when the inituser name is unicode · 0cfe6090
      Tatuya Kamada authored
      Zope4 inituser is decoded by decode('utf-8') on Zope startup and it is unicode on python2,
      therefore it raises UnicodeDecodeError when searching non ascii charater on catalog.
      
        Traceback (most recent call last):
        File "<portal_components/test.erp5.testERP5Catalog>", line 4144, in testSearchNonAsciiWithTheInitUser
          person_module.searchFolder(title=person_title)]
        File "./erp5/product/ERP5Type/Core/Folder.py", line 452, in searchFolder
          return self.portal_catalog.searchResults(**kw)
        File "./erp5/product/ERP5Catalog/CatalogTool.py", line 819, in searchResults
          return ZCatalog.searchResults(self, sql_catalog_id=catalog_id, **kw)
        File "./erp5/product/ZSQLCatalog/ZSQLCatalog.py", line 1070, in searchResults
          return catalog.searchResults(REQUEST, **kw)
        File "./erp5/product/ZSQLCatalog/SQLCatalog.py", line 2362, in searchResults
          **kw
        File "./erp5/product/ZSQLCatalog/SQLCatalog.py", line 2326, in queryResults
          **kw
        File "./erp5/product/ZSQLCatalog/SQLCatalog.py", line 2214, in buildSQLQuery
          only_group_columns,
        File "./erp5/product/ZSQLCatalog/SQLExpression.py", line 397, in asSQLExpressionDict
          'where_expression': self.getWhereExpression(),
        File "./erp5/product/ZSQLCatalog/SQLExpression.py", line 298, in getWhereExpression
          result = self.sql_expression_list[0].getWhereExpression()
        File "./erp5/product/ZSQLCatalog/SQLExpression.py", line 303, in getWhereExpression
          result = '(%s)' % (operator.join(x.getWhereExpression() for x in self.sql_expression_list), )
        UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 25: ordinal not in range(128)
      0cfe6090
    • Jérome Perrin's avatar
      xhtml_style: don't set content type header if already set · 3c8fcd51
      Jérome Perrin authored
      This is necessary on Zope4 because the error page is executed even if
      the response already has a locked body - and in this case we don't
      want to override the content-type header that might also have been set.
      
      One user of this is Base_redirect from erp5_hal_json_style, when
      abort_transaction is True, in that case it sets a body in json, a
      content type header of application/json and then raise a redirect so
      that the transaction is not commited. This fixes a problem visible on
      Zope4 with erp5_bank_reconciliation_renderjs_ui_test:testFunctionalRJSBankReconciliationAction
      3c8fcd51
    • Jérome Perrin's avatar
      hal_json_style/Base_redirect: pass an URL to Redirect · f9fe3a25
      Jérome Perrin authored
      Even if this is not used because we have locked a status earlier, the
      constructor expects an URL with zExceptions 4
      f9fe3a25
  8. 24 Aug, 2022 9 commits
  9. 23 Aug, 2022 4 commits
  10. 22 Aug, 2022 4 commits
    • Jérome Perrin's avatar
      xhtml_style: remove hashes from icons.png URLs · 1b4d4ba9
      Jérome Perrin authored
      this change was made with:
      
          find product/ERP5/bootstrap/erp5_xhtml_style/SkinTemplateItem/portal_skins/erp5_ckeditor/ckeditor/ -type f  | xargs sed -i 's/\\?t=445cf24ebd//g'
      1b4d4ba9
    • Jérome Perrin's avatar
      xhtml_style: remove timestamp from ckeditor URLs · 6e8cc3c9
      Jérome Perrin authored
      This timestamp break officejs offline capabilities.
      
      This reaplies 40f6c8fe ([erp5_xhtml_style] ckeditor: drop hardcoded
      timestamp parameter in URL, 2017-10-16) on the updated CKEditor
      
      This was done with:
      
          find product/ERP5/bootstrap/erp5_xhtml_style/SkinTemplateItem/portal_skins/erp5_ckeditor/ckeditor/ -type f  | xargs sed -i 's/"M6K9"/""/g'
      
      M6K9 was the timestamp for this build, that we can see at the top of
      ckeditor.js
      6e8cc3c9
    • Jérome Perrin's avatar
      c3f769b4
    • Jérome Perrin's avatar
      xhtml_style: version up ckeditor 4.19.1 · 5e82e22a
      Jérome Perrin authored
      done with an external method to upload the content of zip file in
      skin folder.
      
          import zipfile
          from io import BytesIO
      
          def uploadZip(self):
            dest = self.getPortalObject().portal_skins.custom
            with zipfile.ZipFile(
                '/srv/slapgrid/slappart3/srv/project/erp5/ckeditor_4.19.1_4b98d281bc34.zip'
            ) as f:
              for m in f.namelist():
                current_dest = dest
                print(m)
                for part in m.split('/')[:-1]:
                  if part not in current_dest.objectIds():
                    current_dest.manage_addProduct['OFSP'].manage_addFolder(id=part)
                  current_dest = current_dest[part]
                fname = m.split('/')[-1]
                if fname == '_translationstatus.txt':
                  fname = 'translationstatus.txt'
                current_dest.manage_addProduct['OFSP'].manage_addFile(
                  id=fname, file=BytesIO(f.read(m)))
            return repr(dest)
      5e82e22a
  11. 19 Aug, 2022 1 commit
  12. 18 Aug, 2022 3 commits
    • Jérome Perrin's avatar
      Listbox,SelectionTool: use make_query instead of crafting URL manually · 3cb786cc
      Jérome Perrin authored
      Using make_query makes sure that the query parameters are properly
      encoded and also change selection_index to become an int, because
      it keeps the type of parameters.
      
      As a consequence, we had to adjust a few place in the code where
      selection_index was tested for truthiness: because "0" as a string is
      true, but 0 as an int is not. For that, we changed to test the presence
      of selection_name instead of testing selection_index, as they are always
      used together.
      
      This fixes a problem that &amp; in URL was encoded twice for listbox
      anchor links ( bug_module/1137 )
      3cb786cc
    • Jérome Perrin's avatar
      *: reorganise indexation methods · 2bd1d4ed
      Jérome Perrin authored
      02011d8e (immediateReindexObject: use super user to reindex script,
      2015-12-14) did not apply for inventory, because they were overloading
      immediateReindexObject.
      Introduce a new level, _immediateReindexObject that will hold the actual
      reindexing logic.
      
      Previously this method was using PortalContent.reindexObject which was
      monkey patched, to make things less complex and more future proof, move
      the monkey patch to a method on base class.
      
      This also drops alternateReindexObject on BalanceTransaction, because it
      is already defined in Inventory
      2bd1d4ed
    • Jérome Perrin's avatar
      ERP5TypeLiveTestCase: close request at the end of each test · e605544e
      Jérome Perrin authored
      This is supposed to fix "Should not load state for ${oid of a skin} when
      the connection is closed" sometimes happening with live tests (especially
      when the test self.publish and the developer access the site while the
      test is suspended on a debugger breakpoint).
      
      The object accessed after the connection is closed was a skin (python
      script, sometimes page template or form) that was cached in SKINDATA.
      The mechanism to prune entries from the cache uses REQUEST.hold API
      which expects that REQUEST.close is called on request, but because
      requests were not closed at the end of the request, it happened that
      the cache was reused from another connection.
      
      This change to close the requests, like ERP5TypeTestCase is doing in
      tearDown (the actual close is done by Testing.ZopeTestCase.connections).
      
      By closing requests at the end of tests, we also have to change so that
      at the beginning of the test we initialize the request, by using the same
      setSite and setupCurrentSkin that are done in ERP5TypeTestCase.
      e605544e