Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable the system admin to provide complex index settings for Elasticsearch. #961

Closed
svenoe opened this issue Apr 23, 2021 · 10 comments
Closed
Assignees
Labels
contribution Contribution from other parties - thank you! :) enhancement New feature or request
Milestone

Comments

@svenoe
Copy link
Contributor

svenoe commented Apr 23, 2021

@io-architect provided us with a pull request greatly enhancing the ability to customize Elasticsearch. If sensible we should provide one or two more options to adapt stuff in a simpler form via the SysConfig. (#938)

Please use the branch issue-#961-elasticsearch_index for further development.

@svenoe svenoe added enhancement New feature or request contribution Contribution from other parties - thank you! :) labels Apr 23, 2021
@svenoe svenoe added this to the OTOBO 10.0.11 milestone Apr 23, 2021
svenoe added a commit to io-architect/otobo that referenced this issue Apr 23, 2021
@bschmalhofer bschmalhofer modified the milestones: OTOBO 10.0.11, OTOBO 10.0 Jul 3, 2021
@kess-net
Copy link
Contributor

kess-net commented Jul 5, 2021

Tested and enhanced a little bit. Branch issue-#961-elasticsearch_index looks is looking good

@kess-net
Copy link
Contributor

kess-net commented Jul 5, 2021

merged to rel-10_0

@kess-net kess-net closed this as completed Jul 5, 2021
@bschmalhofer bschmalhofer reopened this Jul 5, 2021
@bschmalhofer
Copy link
Contributor

bschmalhofer commented Jul 5, 2021

Reopen for fixing scripts/test/Config/Defaults.t .
@kess-net : Could you take a look at the test? It looks like there are divergent settins in Defaults.pm and Framework.xml.

@kess-net
Copy link
Contributor

kess-net commented Jul 6, 2021

@bschmalhofer , this test is giving me an 'All tests successful.'. What is the problem at your side?

@bschmalhofer
Copy link
Contributor

I got a:

Failed test 'Elasticsearch::ArticleIndexCreationSettings must be the same in Defaults.pm and and setting default value (is not equal, see below)

Diff:
+--+------------------------------+--+------------------+
| |Actual data | |Expected data |
+--+------------------------------+--+------------------+
| 1|$VAR1 = { | 1|$VAR1 = { |

  • 2| 'FieldsLimit' => '2000', * | |
    | 3| 'NR' => '0', | 2| 'NR' => '0', |
    | 4| 'NS' => '1' | 3| 'NS' => '1' |
    | 5| }; | 4| }; |
    +--+------------------------------+--+------------------+

Actual data:
$VAR1 = {
'FieldsLimit' => '2000',
'NR' => '0',
'NS' => '1'
};

Expected data:
$VAR1 = {
'NR' => '0',
'NS' => '1'
};

'

at scripts/test/Config/Defaults.t line 194.

after I did a bin/otobo.Console.pl Maint::Config::Rebuild.

Maybe it's an effect of using bin/docker/quick_setup.pl instead of using installer.pl.

@bschmalhofer
Copy link
Contributor

I have looked at that in the rel-10_0 branch. IMHO the test is correct in reporting a failure, as the config for Elasticsearch::ArticleIndexCreationSettings are indeed diverging between Defaults.pm and Framework.xml. The deeper issue is that this is a necessary divergence. Installer.pm expects a global config for all indexes while the command Maint::ElasticSearch::Migration expects an index specific config, with the default config for 'all'.

I propose that one of these altternatives is implemented:

  1. bring the config for Installer.pm in line with Maint::ElasticSearch::Migration
  2. same as 1., buta visa versa
  3. keep the configs apart, but then at least give them distinctive names

For the record, here is the error from Defaults.pm:

ERROR: OTOBO-otobo.UnitTest-10 Perl: 5.32.0 OS: linux Time: Mon Jul 12 12:52:36 2021

Message: Setting Elasticsearch::ArticleIndexCreationSettings is invalid!

Traceback (3939):
Module: Kernel::System::SysConfig::SettingGet Line: 200
Module: /opt/otobo/scripts/test/Config/Defaults.t Line: 185

Failed test 'Elasticsearch::ArticleIndexCreationSettings must be the same in Defaults.pm and and setting default value (is not equal, see below)

Diff:
+--+------------------------------+--+-----------------+
| |Actual data | |Expected data |
+--+------------------------------+--+-----------------+

  • 1|$VAR1 = { * 1|$VAR1 = \undef; *
  • 2| 'FieldsLimit' => '2000', * | |
  • 3| 'NR' => '0', * | |
  • 4| 'NS' => '1' * | |
  • 5| }; * | |
    +--+------------------------------+--+-----------------+

Actual data:
$VAR1 = {
'FieldsLimit' => '2000',
'NR' => '0',
'NS' => '1'
};

Expected data:
$VAR1 = \undef;

'

at /opt/otobo/scripts/test/Config/Defaults.t line 193.

@svenoe
Copy link
Contributor Author

svenoe commented Jul 12, 2021

It has to be option 1., and it should be noted that adaptions to the index can be made in Config.pm before running the installer.

@bschmalhofer
Copy link
Contributor

Another tidbit: mayber rename 'all' to something like 'default' or 'standard'. The config is no longer for all indexes when there is a specific config.

kess-net added a commit that referenced this issue Aug 5, 2021
Introduce separeate settings for each elasticserach index.
kess-net added a commit that referenced this issue Aug 6, 2021
Fix Migration.pm and correct test accordinly.
Also use Test2:V0 directly
kess-net added a commit that referenced this issue Aug 7, 2021
Introduce separeate settings for each elasticserach index.
kess-net added a commit that referenced this issue Aug 7, 2021
Fix Migration.pm and correct test accordinly.
Also use Test2:V0 directly
kess-net added a commit that referenced this issue Aug 7, 2021
minor readability changes
kess-net added a commit that referenced this issue Aug 9, 2021
kess-net added a commit that referenced this issue Aug 11, 2021
Template and settings are predefined in Kernel/Config/Defaults.pm for
the indices Customer, CustomerUser and Ticket.
kess-net added a commit that referenced this issue Aug 12, 2021
kess-net added a commit that referenced this issue Aug 12, 2021
Maint::Elasticsearch::Migration now uses Index templates the same way as
the installer
kess-net added a commit that referenced this issue Aug 15, 2021
…ault if no Elasticsearch::IndexSettings is active
kess-net added a commit that referenced this issue Aug 15, 2021
kess-net added a commit that referenced this issue Aug 15, 2021
Also
 * moved two helper methods from
 Kernel/System/Console/Command/Maint/Elasticsearch/Migration.pm to
 Kernel/System/Elasticsearch.pm
 * adapted a test script
kess-net added a commit that referenced this issue Aug 17, 2021
commit 8782cd5ab4c50c21cf70d78cb26fb601dbfa1108
Author: kess <kess-net@users.noreply.github.com>
Date:   Thu Aug 12 18:44:48 2021 +0200

    issue #961 adapt test to latest code changes

commit dd6db92869d23168071b4bca9bdbf14084f723ea
Author: kess <kess-net@users.noreply.github.com>
Date:   Thu Aug 12 17:57:09 2021 +0200

    make CodePolicy.pl happy

commit 1629e98a269643110671a1e5f9c7e4ad99e2c54d
Author: kess <kess-net@users.noreply.github.com>
Date:   Thu Aug 12 17:09:16 2021 +0200

    issue #961 enablle index setting templates

    Maint::Elasticsearch::Migration now uses Index templates the same way as
    the installer

commit b52eca0a72243893df267a28b219d4df7305fb2b
Author: kess <kess-net@users.noreply.github.com>
Date:   Thu Aug 12 09:04:16 2021 +0200

    issue #961 fix variable name

commit ebb7dda556682e83a538ef7631ce002433442911
Author: kess-net <kess-net@users.noreply.github.com>
Date:   Wed Aug 11 08:36:23 2021 +0200

    issue #961 Installer now respects customizable index settings

    Template and settings are predefined in Kernel/Config/Defaults.pm for
    the indices Customer, CustomerUser and Ticket.

commit 50358e30942e10cba3ec4ac5e223a2118bbd6006
Author: kess-net <kess-net@users.noreply.github.com>
Date:   Mon Aug 9 23:04:59 2021 +0200

    Code style

    use the logical defined or operator for better readability

commit c49a7b74584f6535aae907aef5055da2125ccf00
Author: kess-net <kess-net@users.noreply.github.com>
Date:   Mon Aug 9 22:39:03 2021 +0200

    issue #961,  PR 1165 the installer now users FieldsLimit setting

commit ea0b810508a0ed825a7c0d63aced517c021674af
Author: kess-net <kess-net@users.noreply.github.com>
Date:   Mon Aug 9 13:51:50 2021 +0200

    ssue #961 - PR 1165 be strict here

commit 5b52d69974b4125969608b4e705cea56cd3fc3a9
Author: kess-net <kess-net@users.noreply.github.com>
Date:   Sat Aug 7 17:48:52 2021 +0200

    issue #961 elasticsearch_index

    minor readability changes

commit 04214b4de22469498c31e042a349c1c1b0451972
Author: kess-net <kess-net@users.noreply.github.com>
Date:   Fri Aug 6 14:14:10 2021 +0200

    issue #961 elasticsearch_index

    Fix Migration.pm and correct test accordinly.
    Also use Test2:V0 directly

commit edd274bfdf69efdb154025b7e6c83be47e857599
Author: kess-net <kess-net@users.noreply.github.com>
Date:   Thu Aug 5 15:06:52 2021 +0200

    issue #961 elasticsearch index

    Introduce separeate settings for each elasticserach index.
kess-net added a commit that referenced this issue Aug 17, 2021
- Index settings may be changed in the system configuration
  Old 'ArticleIndexCreationSettings' for all indices is deprecated but
  still valid. It will be replaced by 'IndexSettings###Default'. Separate
  settings are possible for the indices 'customer', 'customeruser' and
  'ticket'
- index templates are used for the indices 'customer', 'customeruser'
  and 'ticket'
- index templates may be configured in the Defaults.pm
- adapted test script

Also
- number values in index templates shall not be overruled by index settings
- moved two helper methods from
 Kernel/System/Console/Command/Maint/Elasticsearch/Migration.pm to
 Kernel/System/Elasticsearch.pm
svenoe added a commit that referenced this issue Aug 18, 2021
Solved issue #961 improve elasticsearch index definition settings
kess-net added a commit that referenced this issue Aug 19, 2021
svenoe added a commit that referenced this issue Aug 20, 2021
svenoe added a commit that referenced this issue Aug 20, 2021
Issue #961: Tests don't like settings not activated yet
@svenoe svenoe closed this as completed Aug 20, 2021
@bschmalhofer
Copy link
Contributor

The commit b0c99a8 by @kess-net did not make it into rel-10_0. This commit was moving the SysConfix XML snippet for Elasticsearch::IndexSettings###ConfigItem into the ITSMConfigurationManagement package. Apparently it was decided to keep it in OTOBO core.

@bschmalhofer
Copy link
Contributor

There is also an old branch by @kess-net , https://github.com/RotherOSS/otobo/tree/issue-%23961-development_by_kess. This looks like useful changes, but nothing that is essential. Let's keep that branch for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution Contribution from other parties - thank you! :) enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants