Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-28856

Spider: Implement more engine-defined options

Details

    Description

      Implement the engine-defined options corresponding to useful table
      options, including (options marked as ++ are included only because
      of the "keep" decision on the corresponding spider system variables
      in MDEV-27228):

      • auto_increment_mode
      • bgs_mode
      • bulk_size
      • bulk_update_size
      • connect_timeout
      • multi_split_read
      • net_read_timeout
      • net_write_timeout
      • priority
      • query_cache
      • query_cache_sync
      • read_only_mode
      • skip_parallel_search++
      • host
      • password
      • port
      • username
      • wrapper
      • default_file
      • default_group
      • driver
      • dsn
      • filedsn
      • socket
      • ssl_capath
      • ssl_ca
      • ssl_cert
      • ssl_cipher
      • ssl_key
      • ssl_vscs
      • use_pushdown_udf
      • force_bulk_delete
      • force_bulk_update
      • table_count_mode
      • delete_all_rows_type
      • idx

      Spider table parameters are documented (incompletely) at
      https://mariadb.com/kb/en/spider-table-parameters/

      Many of these table params have a twin system variable. Examples
      include read_only_mode, net_read_timeout etc. The natural
      approach to these params is to use HA_TOPTION_SYSVAR macro,
      which takes care of the value overriding. This is also a natural
      continuation of spider sysvar tickets like MDEV-27169 and
      MDEV-31524. However, table options can not fall back dynamically to
      the current sysvar value, so we have to keep the overriding/fallback
      mechanism and use the string type (HA_TOPTION_STRING) for these
      values, where we can encode unspecified as the null string.

      There's also a temporary problem with unspecified non-string table
      options. Given that we will keep connection string parsing by
      COMMENT or CONNECTION until MDEV-31146, It makes sense as a part of
      the changes for this ticket, to ignore COMMENT/CONNECTION parsing
      when any table option is used. However, it is not possible to tell
      whether a non-string type table option has been specified. We fix
      this by making these options also string type (so all spider options
      are of the string type) and if any of them is non-null (i.e.
      specified), we ignore COMMENT/CONNECTION for parsing. An alternative
      fix also considered was to make the COMMENT string parsing override
      these table options. However, this is an inferior choice given
      that 1. this increases complexity in overriding; 2. only six
      relatively uncommon table options are not sysvars and not strings
      (priority, query_cache, query_cache_sync, force_bulk_delete,
      force_bulk_update, table_count_mode); 3. all params are already
      supplied as strings (e.g. read_only_mode "1") in the existing
      comment string parsing so no surprise to users if they remain
      strings, and in create table statements strings do not need to be
      quoted anyway; 4. users might want to use comments for comments.

      Even though after this task there are MDEV-28861 and MDEV-31146 that
      duplicate and remove connection info encoded in comments, there will
      still be connection string parser in the spider codebase, for spider
      udfs like spider_direct_sql and spider_copy_tables.

      Regarding the choice of the table options in the list above, some
      undocumented parameters have unclear purpose. It is not in the scope
      of this ticket to try to document every one of them. Examples
      include force_bulk_update, force_bulk_delete,
      table_count_mode. Also worth noting is the parameter
      delete_all_rows_type, which is buggy at 11.2 (MDEV-31996), but
      on face value its purpose is clear, so we keep it for now.

      Attachments

        Issue Links

          Activity

            Roel Roel Van de Paar added a comment - - edited

            Status update: many new issues were seen during the testing of this feature. Most of these were traced back to 1) missing MDEV-31524 and MDEV-30981 commits in the feature branch, 2) MDEV-32492 (and the iterations thereof, ref bug for details), which turned out to be a legacy bug which asserted itself more readily in the feature branch. The final re-test results of the updated branch of this feature (bb-11.3-mdev-28856-and-fixes) are near complete.

            Roel Roel Van de Paar added a comment - - edited Status update: many new issues were seen during the testing of this feature. Most of these were traced back to 1) missing MDEV-31524 and MDEV-30981 commits in the feature branch, 2) MDEV-32492 (and the iterations thereof, ref bug for details), which turned out to be a legacy bug which asserted itself more readily in the feature branch. The final re-test results of the updated branch of this feature (bb-11.3-mdev-28856-and-fixes) are near complete.

            Both MDEV-32558 (marked as duplicate of MDEV-32492 as the underlying issue is the same) as well as the newly created MDEV-32585 are bugs that show in optimized builds only. For MDEV-32558 after discussion we agreed it is acceptable to leave it and fix later in combination with MDEV-32492. MDEV-32585 is a WIP.

            Roel Roel Van de Paar added a comment - Both MDEV-32558 (marked as duplicate of MDEV-32492 as the underlying issue is the same) as well as the newly created MDEV-32585 are bugs that show in optimized builds only. For MDEV-32558 after discussion we agreed it is acceptable to leave it and fix later in combination with MDEV-32492 . MDEV-32585 is a WIP.

            OK to push.

            Roel Roel Van de Paar added a comment - OK to push.
            ycp Yuchen Pei added a comment -

            Thanks for the testing Roel.

            Given the patches depend on the MDEV-31117 commits, the latter having
            not yet reached 11.3 (highest version inclusion 11.1), the push is
            blocked by the merge of 11.1->11.2->11.3.

            ycp Yuchen Pei added a comment - Thanks for the testing Roel . Given the patches depend on the MDEV-31117 commits, the latter having not yet reached 11.3 (highest version inclusion 11.1), the push is blocked by the merge of 11.1->11.2->11.3.
            ycp Yuchen Pei added a comment - - edited

            squashed the following and pushed to 11.3 as 126157061b4376496c034a809ea4943e863d1465

            cc08a83ef42 * MDEV-32486 Fix duplicate spider_bulk_malloc ID
            99c3003ec78 * MDEV-32234 Fix missing space in Spider variable descriptions
            80e4b349fc2 * MDEV-28856 fixes test to pass in --ps-protocol
            a4031e4c051 * MDEV-28856 Add remaining Spider table options
            

            ycp Yuchen Pei added a comment - - edited squashed the following and pushed to 11.3 as 126157061b4376496c034a809ea4943e863d1465 cc08a83ef42 * MDEV-32486 Fix duplicate spider_bulk_malloc ID 99c3003ec78 * MDEV-32234 Fix missing space in Spider variable descriptions 80e4b349fc2 * MDEV-28856 fixes test to pass in --ps-protocol a4031e4c051 * MDEV-28856 Add remaining Spider table options

            People

              ycp Yuchen Pei
              nayuta-yanagisawa Nayuta Yanagisawa (Inactive)
              Votes:
              1 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.