-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
FIX for issue 9930 - Asymmetric Transaction Error with ElasticSearch #10610
FIX for issue 9930 - Asymmetric Transaction Error with ElasticSearch #10610
Conversation
Hi @phoenix128! I'll be checking this PR, will let you know if we have any questions/feedback. Thanks for your contribution! |
Hi @miguelbalparda , |
@@ -193,6 +193,7 @@ public function addImage( | |||
$mediaGalleryData['images'][] = [ | |||
'file' => $fileName, | |||
'position' => $position, | |||
'media_type' => 'image', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @phoenix128 , This PR makes sense for Magento 2.1.x , because in Magento 2.2.0
We've deprecated interface
namespace Magento\Elasticsearch\Model\Adapter;
/**
* @deprecated
* @see \Magento\Elasticsearch\Model\Adapter\BatchDataMapperInterface
*/
interface DataMapperInterface
and added \Magento\Elasticsearch\Model\Adapter\BatchDataMapperInterface
instead.
In the implementation of new ProductDataMapper we don't support handling and persisting Media attribute types and adding them to ES index.
namespace Magento\Elasticsearch\Model\Adapter\BatchDataMapper;
/**
* Map product index data to search engine metadata
*/
class ProductDataMapper implements BatchDataMapperInterface
{
/**
* List of attributes which will be skipped during mapping
*
* @var array
*/
private $defaultExcludedAttributes = [
'price',
'media_gallery',
'tier_price',
'quantity_and_stock_status',
'media_gallery',
'giftcard_amounts',
];
/**
* {@inheritdoc}
*/
public function map(array $documentData, $storeId, array $context = [])
{
// reset attribute data for new store
$this->attributeData = [];
$documents = [];
foreach ($documentData as $productId => $indexData) {
$this->builder->addField('store_id', $storeId);
$productIndexData = $this->convertToProductData($productId, $indexData, $storeId);
foreach ($productIndexData as $attributeCode => $value) {
if (in_array($attributeCode, $this->excludedAttributes, true)) {
continue;
}
$this->builder->addField(
$this->fieldMapper->getFieldName(
$attributeCode,
$context
),
$value
);
}
$documents[$productId] = $this->builder->build();
}
Regarding PR in 2.1.x - I propose to slightly modify ProductDataMapper along with your change as well, because the real root cause of the problem is Magento\Elasticsearch\Model\Adapter\DataMapper\ProductDataMapper
protected function getProductMediaGalleryData($media, $roles)
{
$result = [];
if (!empty($media['images'])) {
$i = 0;
foreach ($media['images'] as $data) {
if ($data['media_type'] === 'image') {
$result['image_file_' . $i] = $data['file'];
$result['image_position_' . $i] = $data['position'];
$result['image_disabled_' . $i] = $data['disabled'];
$result['image_label_' . $i] = $data['label'];
$result['image_title_' . $i] = $data['label'];
$result['image_base_image_' . $i] = $this->getMediaRoleImage($data['file'], $roles);
$result['image_small_image_' . $i] = $this->getMediaRoleSmallImage($data['file'], $roles);
$result['image_thumbnail_' . $i] = $this->getMediaRoleThumbnail($data['file'], $roles);
$result['image_swatch_image_' . $i] = $this->getMediaRoleSwatchImage($data['file'], $roles);
} else {
$result['video_file_' . $i] = $data['file'];
$result['video_position_' . $i] = $data['position'];
$result['video_disabled_' . $i] = $data['disabled'];
$result['video_label_' . $i] = $data['label'];
$result['video_title_' . $i] = $data['video_title'];
$result['video_base_image_' . $i] = $this->getMediaRoleImage($data['file'], $roles);
$result['video_small_image_' . $i] = $this->getMediaRoleSmallImage($data['file'], $roles);
$result['video_thumbnail_' . $i] = $this->getMediaRoleThumbnail($data['file'], $roles);
$result['video_swatch_image_' . $i] = $this->getMediaRoleSwatchImage($data['file'], $roles);
$result['video_url_' . $i] = $data['video_url'];
$result['video_description_' . $i] = $data['video_description'];
$result['video_metadata_' . $i] = $data['video_metadata'];
$result['video_provider_' . $i] = $data['video_provider'];
}
$i++;
}
}
return $result;
}
Currently there are two types of media content in Magento :
external-video
and image
I propose to add next code to the above function:
if ($data['media_type'] === 'image') {
// some logic here
} elseif ($data['media_type'] === 'external-video') {
// some logic here
} else {
throw new Magento\Framework\Exception\InputExcaption('Unsupported Media Type Provided');
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maghamed I don't know where the source code is for the elastic search stuff, but since the defaultExcludedAttributes are private, is there a way to change them or override them? I want to include price and media gallery, but since its private and not protected, I don't know a way around this.
taking above into account I propose to close this PR and create new one for 2.1.x Magento branch |
Hello @maghamed and nice to see you again ;) Sounds right to move to 2.1.x only, I did not check this against 2.2.x . BTW: The problem is not in the handling itself, but in the undefined |
Reopening after a short analysis with @maghamed in a separate slack channel. The real problem is coming from |
My fault sorry, PR makes sense for me. Thanks, @phoenix128 for your contribution! |
@maghamed are you sure PRs to |
IMHO this bug is affecting the 2.2 develop branch too. In 2.1 EE results into an error, but in 2.2 it remains, even if potential. |
[EngCom] Public Pull Requests - MAGETWO-71833: Grammar fix for #9533 #10627 - MAGETWO-71801: FIX for issue 9930 - Asymmetric Transaction Error with ElasticSearch #10610 - MAGETWO-71762: Bug fix, prevent displaying 0000-00-00 00:00:00 as anything else in admin grids #10598 - MAGETWO-71532: Fix swagger-ui on instances of Magento running on a non-standard port #10504
[Plankton] Fixes - MAGETWO-80191: [2.2.x] - Fixes #10255: base_shipping_discount_tax_compensation_amnt was empty from order onwards #10435 - MAGETWO-80203 [2.2.x]: - FIX for issue 9930 - Asymmetric Transaction Error with ElasticSearch #10610 - MAGETWO-62981: Cannot save category position after drag&drop (second attempt) - MAGETWO-82339: Duplicate Shipment getting created
Asymmetric Transaction error while adding products promatically with ElasticSearch.
Description
While using ElasticSearch as search engine, we may get an "Asymmetric Trasnsaction" error on reindex.
This is caused by an undefined index trying to get
media_type
value from the image attribute.This does not happen while uploading images via browser, but it happens adding products programmatically and using
\Magento\Catalog\Model\Product\Gallery\Processor::addImage
core method.Fixed Issues
Manual testing scenarios
Contribution checklist