Развалились InventSum - InventTrans

[Logger]

Обнаружил интересную особенность :
Код обновляющий, InventSum (если быть точнее - пишущий данные об изменения в InventsumDelta - но в данном случае это непринципиально)
для методов
InventTrans.insert()
InventTrans.delete()

расположен после Super()
а для
InventTrans.update()
до Super()

что приводит в ряде случаев к тому, что расходятся данные InventSum - InventTrans.

Например :
в коде есть вызов такого типа :

            ...
            ttsBegin;
            try
            {
                // здесь расположен код обновляющий InventTrans - например комплектация или резервирование
            }
            catch(Exception::Error)
            {
                // обработка                    
            }
            catch 
            {   // сюда попадает обработка конфликта обновления записи
                // ничего не делаем
            }
            ttsCommit;
            ...

В случае если на обновлении InventTrans в блоке try случится исключительная ситуация конфликта обновления записи, то код пишуший
информацию в InventSumDelta успеет выполниться и транзакция не откатывается (такова особенность этого исключения).
Далее после коммита транзакции изменения сбрасываются в InventSum, а Inventtrans не изменился.
Получаем расхождения.

Можно конечно возразить что пример несколько искусственный, но мы на практике с этим столкнулись. Получается, что отсутствие отката транзакции при Exception::UpdateConflict - достаточно опасная вещь. По крайней, мере лучше всегда этот тип исключения обрабатывать.

А еще лучше поправить метод InventTrans.update(), переместив обновление InventSumDelta после вызова super(). Это будет дополнительной защитой от небрежно написанного кода. Правда все последствия такого переноса пока не изучил.

Как думаете, рискуем чем нибудь ?

[CDR]

Цитата:

Сообщение от Logger

Это будет дополнительной защитой от небрежно написанного кода.

Улыбнула "защита от небрежно написанного кода" .

Если есть две обновляемые записи, то тут по-моему, все равно, какую из них первой обновлять, при наличии "небрежно написанного кода".

Если описанная ситуация действительно для Вас актуальна, то можно порекомендовать вставить блок try-catch внутрь методов insert(), update(), delete(). Тогда при исключениях, которые не откатывают транзакцию автоматически, в catch можно будет выбросить исключение Exception::Error и откатить транзакцию принудительно.

[lvan]

Цитата:

Сообщение от CDR

можно порекомендовать вставить блок try-catch внутрь методов insert(), update(), delete().

а ничо, что try..catch внутри транзакции не работает?
автору просто надо завернуть обновление inventSum и super() в транзакцию
вообще странно, что её там нет

[lvan]

хотя я про updateConflict не подумал, но кстати обычно он всегда обрабатывается
в Tutorial_RunbaseBatch есть правильная конструкция

/// <summary>
///    Contains the code that does the actual job of the class.
/// </summary>
public void run()
{
    #OCCRetryCount
    if (! this.validate())
        throw error("");

    try
    {
        ttsbegin;

        // this.Update();

        ttscommit;
    }
    catch (Exception::Deadlock)
    {
        retry;
    }
    catch (Exception::UpdateConflict)
    {
        if (appl.ttsLevel() == 0)
        {
            if (xSession::currentRetryCount() >= #RetryNum)
            {
                throw Exception::UpdateConflictNotRecovered;
            }
            else
            {
                retry;
            }
        }
        else
        {
            throw Exception::UpdateConflict;
        }
    }

}

[Ivanhoe]

Logger, вы про какую версию системы пишите? Ошибка происходит на стандартной функциональности или есть кастомизации?

[Logger]

2009-я Аксапта.
Кастомизаций конечно дофига, но в части разноски данных в InventSum мы ничего не меняли.
Ближе к вечеру выложу демонстрационный джоб, чтобы на любой инсталляции можно было глюк проверить.

[Pustik]

Цитата:

Сообщение от Logger

Можно конечно возразить что пример несколько искусственный, но мы на практике с этим столкнулись.

Мы тоже с этим столкнулись. Это происходит периодически, но не постоянно.Вот в чем у нас вопрос. Меня периодически просят разобраться, почему по оборотам одно количество, по кнопке "в наличии" другое. Пересчет InventSum по номенклатуре стандартным образом возвращает все на свое место. В большинстве своем - все ок.
Logger, спасибо за гипотезу наших парадоксов).

[Logger]

Вот пример джоба :

// GRD_R4453_InventSumInventTrans_pkoz, Разъехались остатки в InventSum и проводки в InventTrans., pkoz, 04.11.2011
static void Job791_15(Args _args)
{
    #define.recIdTrans(5637151827)
    InventTrans     InventTrans;
    InventSum       InventSum;
    ;
    try
    {
        ttsBegin;
        try
        {
            InventTrans = InventTrans::findRecId(#recIdTrans, true);
            InventTrans.Qty += 1.0;
            InventTrans.Update();
            breakpoint;
        }
        catch
        {
            info("поймали внутрений catch");
        }
        ttsCommit;
    }
    catch
    {
        info("поймали внешний catch");
    }

    info(strFMT("InventTrans.RowCount() = %1, InventTrans.Qty = %2, InventSum : %3 ",
        InventTrans.RowCount(),
        InventTrans.Qty,
        con2str(Global::buf2Con(InventSum::find(InventTrans.ItemId, InventTrans.inventDimId)), ";  ")
        ));

}

Использовать так :
1. Проставляем в джобе recID существующей проводки InventTrans
2. Открываем 2 клиента аксапты.
3. Запускаем джоб на 1-м клиенте, ждем когда он выпадет в отладчик на точке останова.
4. Запускаем тот же джоб на 2-м клиенте. Он повиснет.
5. Жмем F5 в отладчике на 1-м клиенте.

В итоге после выполнения джоба на обоих клиентах, получаем что InventTrans.Qty увеличилось на 1, а соответствующая колонка в InventSum увеличилась на 2.

Причина такого поведения, в том что мы просто поставили обработку catch в которую попадают исключения
Exception::UpdateConflict
и
Exception:uplicateKeyException
которые в свою очередь не откатывают транзакцию.

Поэтому чтобы избегать в модификациях подобных ошибок, нельзя ставить обработку catch без указания вида исключения.
Ну или по крайней мере перед ней всегда делать обработку вышеупомянутых типов исключений.

Для удобства, я завел макрос Catch_DangerousException, который можно везде вставлять в блок обработки.
Текст макроса

catch (Exception::UpdateConflict)
{
    if (appl.ttsLevel() == 0)
    {
        if (xSession::currentRetryCount() >= 5)
        {
            throw Exception::UpdateConflictNotRecovered;
        }
        else
        {
            retry;
        }
    }
    else
    {
        throw Exception::UpdateConflict;
    }
}

catch (Exception::DuplicateKeyException)
{
    // info(strfmt("@SYS123267", ));

    if (appl.ttsLevel() != 0)
    {
        throw Exception::DuplicateKeyExceptionNotRecovered;
    }
}

Упомянутый в заголовке темы пример следует переписать так :

            ...
            ttsBegin;
            try
            {
                // здесь расположен код обновляющий InventTrans - например комплектация или резервирование
            }
            catch(Exception::Error)
            {
                // обработка                    
            }

            #catch_DangerousException

            catch 
            {   // сюда попадает обработка конфликта обновления записи
                // ничего не делаем
            }
            ttsCommit;
            ...

[Logger]

Если вышеупомянутый джоб переписать с использованием макроса по обработке исключений, то упомянутых расхождений InventSum и InventTrans не возникнет.

// GRD_R4453_InventSumInventTrans_pkoz, Разъехались остатки в InventSum и проводки в InventTrans., pkoz, 04.11.2011
static void Job791_16(Args _args)
{
    #define.recIdTrans(5637151827)
    InventTrans     InventTrans;
    InventSum       InventSum;
    ;
    try
    {
        ttsBegin;
        try
        {
            InventTrans = InventTrans::findRecId(#recIdTrans, true);
            InventTrans.Qty += 1.0;
            InventTrans.Update();
            breakpoint;
        }

        #catch_DangerousException

        catch
        {
            info("поймали внутрений catch");
        }
        ttsCommit;
    }

    #catch_DangerousException

    catch
    {
        info("поймали внешний catch");
    }

    info(strFMT("InventTrans.RowCount() = %1, InventTrans.Qty = %2, InventSum : %3 ",
        InventTrans.RowCount(),
        InventTrans.Qty,
        con2str(Global::buf2Con(InventSum::find(InventTrans.ItemId, InventTrans.inventDimId)), ";  ")
        ));

}

[Logger]

Любопытно, что в стандартной документации в разделе Exception Handling про такую особенность исключений ничего не сказано.

Более того написано буквально следующее :

Цитата:

One strategy is to have the last catch statement leave the exception type unspecified. This means it handles all exceptions that are not handled by a previous catch. This strategy is appropriate for the outermost try - catch blocks.

try { /* Code here. */ } 

catch (Exception::Numeric) { info("Caught a Numeric exception."); } 

catch { info("Caught an exception."); }

Т.е. если четко следовать документации, то можно нагородить ошибок.
Хотя формально все правильно, просто в некоторых редких случаях разъезжается InventSum и InventTrans.

Также аналогичная проблема может проявиться в любом месте при конфликте обновления записи и выглядеть это будет так словно транзакция закоммитилась посередине.

[Logger]

Также решил воспользоваться советом CDR и модифицировал методы :
InventTrans.Update()
InventTrans.Insert()
InventTrans.Delete()

поставив тело каждого метода в блок try и написав такой блок catch в каждом методе :

    catch (Exception::UpdateConflict)
    {
        // warning(this.GRD_getWarningStr4UpdateConflict());
        throw Exception::UpdateConflictNotRecovered;
    }

    catch (Exception::DuplicateKeyException)
    {
        // warning(this.GRD_getWarningStr4DuplicateKeyException());
        throw Exception::DuplicateKeyExceptionNotRecovered;
    }

При такой модификации ошибка расхождения InventTrans и InventSum не возникнет, даже используя джоб
Развалились InventSum - InventTrans

Так пожалуй будет надежней.
Нельзя полагаться на то, что никто из разработчиков не ошибется при обработке исключений, для кода содержащего InventTrans.Update()

[Ivanhoe]

Так все-таки, указанная проблема есть в стандартной функциональности? Т.е. вы знаете места, где "некорректно" обрабатываются исключения?

[Logger]

Таких примеров не знаю в стандарте.
Может есть, а может и нет.

Суть проблемы в том, что если следовать стандарту, описанному в документации, то легко написать код, который приведет к проблемам.

Например, если вы посмотрите на приведенный мной джоб, то, наверно, не заподозрите подвоха. А он есть

[Logger]

Поискал по ветке классов в AOT по такому выражению

Цитата:

catch\s*[^( Exception)]

(задумка была такая - отфильтровать в поиске все строки где написано просто

catch

и пропустить те, где после catch стоит упоминание Exception
Данное регулярное выражении при поиске работает не совсем правильно, но большинство ненужных случаев позволяет отбросить.

В общем, вывалилась куча примеров на sys слое когда стоит просто

try
{
...
}
Catch
{
...
}

Конечно не все они приведут к описанному здесь глюку, так как не везде в блоку try стоит обновление данных, кое где стоит вызов com объектов и.т.п.

Но все равно очень много мест где есть обновление данных и потенциально будут описанные в этой ветке проблемы

[Logger]

На эту проблему обратили внимание в Майкрософте
mfp: X++, the catch
Kashperuk Ivan: Tutorial Link: Handling exceptions the right way in X++

Оригинальные ссылки
https://blogs.msdn.microsoft.com/mfp...4/x-the-catch/
http://kashperuk.blogspot.ru/2016/11...ons-right.html

[Link]

Цитата:

Сообщение от Logger

На эту проблему обратили внимание в Майкрософте
mfp: X++, the catch
Kashperuk Ivan: Tutorial Link: Handling exceptions the right way in X++

Оригинальные ссылки
https://blogs.msdn.microsoft.com/mfp...4/x-the-catch/
http://kashperuk.blogspot.ru/2016/11...ons-right.html

Пять лет, Карл! И каков вывод? "Вы просто держите его неправильно." (с)

[Logger]

Цитата:

Сообщение от Link

Пять лет, Карл!

Чёртова бюрократия !
Если бы вовремя донести до нужных лиц эту инфу

[kashperuk]

Цитата:

Сообщение от Logger

Чёртова бюрократия !
Если бы вовремя донести до нужных лиц эту инфу

Да, это оказались довольно опасные грабли, и поведение при наступлении на них может быть весьма и весьма непредсказуемым и, что еще хуже, трудновоспроизводимым.

[Logger]

Говорят в 4-ке баг с транзакциями не воспроизводился.
Кто-нибудь может проверить, это так или нет ?

[dech]

Цитата:

Сообщение от Logger

Обнаружил интересную особенность :
Код обновляющий, InventSum (если быть точнее - пишущий данные об изменения в InventsumDelta - но в данном случае это непринципиально)
для методов
InventTrans.insert()
InventTrans.delete()

расположен после Super()
а для
InventTrans.update()
до Super()

что приводит в ряде случаев к тому, что расходятся данные InventSum - InventTrans.

Недавно натолкнулись на расхождение InventSum/InventTrans в 4-ке.
Подскажите, где вы нашли "безусловный" catch?