vuestorefront / magento2-vsbridge-indexer

This is official Vue Storefront, native, Magento2 indexer
https://vuestorefront.io
MIT License
69 stars 89 forks source link

Lack of mappings for created_at & updated_at #306

Closed Fifciu closed 4 years ago

Fifciu commented 4 years ago

Vue storefront tries to sort items by created_at/updated_at natively. Unfortunately, as these attributes have bad mappings (they're strings) - Elasticsearch throws an error and whole app cannot work.

Error:

[search] Can not connect the vue-storefront-api / ElasticSearch instance! Error: {"code":500,"result":{"root_cause":[{"type":"illegal_argument_exception","reason":"Fielddata is disabled on text fields by default. Set fielddata=true on [created_at] in order to load fielddata in memory by uninverting the inverted index. Note that this can however use significant memory. Alternatively use a keyword field instead."}],"type":"search_phase_execution_exception","reason":"all shards failed","phase":"query","grouped":true,"failed_shards":[{"shard":0,"index":"vue_storefront_catalog","node":"n3ssKVWpQqel2-JaWx-ruw","reason":{"type":"illegal_argument_exception","reason":"Fielddata is disabled on text fields by default. Set fielddata=true on [created_at] in order to load fielddata in memory by uninverting the inverted index. Note that this can however use significant memory. Alternatively use a keyword field instead."}}]}}

Obviously, using keyword modifier is not a solution because with that - it would try to sort alphabetically when we have to sort by Date.

How to reproduce

  1. Prepare VSF-API 1.12 with data in ES imported from magento2-vsbridge-indexer
  2. Open some API Product Catalog endpoint and try to sort by created_at, e.g.: GET /api/catalog/vue_storefront_catalog/product/_search?sort=created_at

These values should work properly (I don't list them with :desc because they will work if these 2 work):

Fifciu commented 4 years ago

I believe some other than Product Indexer (I suspect attribute indexer) creates bad mappings. It might be caused by updated_at and created_at in private $fieldsToRemove.

When I purge my ES and run: php bin/magento indexer:reindex - I am getting mapping error When I purge my ES and run:

// To create proper mappings for these 2 attributes first
php bin/magento indexer:reindex vsbridge_product_indexer
php bin/magento indexer:reindex

It works properly

janmyszkier commented 4 years ago

by default, created_at was not mentioned in https://github.com/DivanteLtd/vue-storefront/blob/master/config/default.json#L640 so by default, this repo is also not supporting this (which is not a bug)

If you want to support non-default fields, you need to alter your indexes and mappings accordingly. This is left to the user, because not every field can be keyword field and it's too memory-consuming to turn fielddata for all fields

more on this: https://www.elastic.co/guide/en/elasticsearch/reference/5.6/fielddata.html#fielddata

Fifciu commented 4 years ago

VSF PWA Default theme uses this attribute for homepage's mixin:

const { items } = await dispatch('product/findProducts', {
        query: newProductsQuery,
        size: 8,
        sort: 'created_at:desc',
        options: {
          populateRequestCacheTags: true,
          prefetchGroupProducts: false
        }
      }, { root: true })

It isn't in the config because PWA does not need it. But we still might want to sort by it without fetching this attribute - same we could do for updated_at.

What's more, we have code that tries to set these mappings for both created_at & updated_at in current master, check GeneralMapping class:

private $commonProperties = [
        'position' => ['type' => FieldInterface::TYPE_LONG],
        'level' => ['type' => FieldInterface::TYPE_INTEGER],
        'created_at' => [
            'type' => FieldInterface::TYPE_DATE,
            'format' => FieldInterface::DATE_FORMAT,
        ],
        'updated_at' => [
            'type' => FieldInterface::TYPE_DATE,
            'format' => FieldInterface::DATE_FORMAT,
        ]
    ];

I believe, there might be some conflict with "removing fields optimization logic" from AttributeData class (as I described in the previous post):

private $fieldsToRemove = [
        'row_id',
        'created_in',
        'updated_in',
        'entity_id',
        'entity_type_id',
        'attribute_set_id',
        'all_children',
        'created_at',
        'updated_at',
        'request_path',
    ];
afirlejczyk commented 4 years ago

I believe, there might be some conflict with "removing fields optimization logic" from AttributeData class (as I described in the previous post):

I don't think so.

Now created_at field should be exported for products: https://github.com/DivanteLtd/magento2-vsbridge-indexer/pull/299/files, but I didn't check if the mapping is still correct. It for sure was, but now we have more indices so I have to double check. I will check between types if the mapping is correct everywhere and make a fix if needed.

Fifciu commented 4 years ago

Looks like it was an issue related to change Node indexer to this one. This is weird because I sent DELETE http://localhost:9200/_all many times. I also reinstalled ES5.6 (I removed es's files) and it still didn't work. I prepared new VPS and everything looks fine for now