moqui / moqui-framework

Use Moqui Framework to build enterprise applications based on Java. It includes tools for databases (relational, graph, document), local and web services, web and other UI with screens and forms, security, file/resource access, scripts, templates, l10n, caching, logging, search, rules, workflow, multi-instance, and integration.
http://www.moqui.org
Other
284 stars 204 forks source link

Date type field is stored in the database,Have concurrency problem #326

Closed ofbizer closed 6 years ago

ofbizer commented 6 years ago

Database using Oracle 11G,50 users tested,date of discovery changed.Later modified the calendar get method,this problem is solved. My current way is to get a new calendar object every time. Is there a concurrency problem with the calendar object? Please forgive me for lame in English.

framework/src/main/groovy/org/moqui/impl/entity/EntityFacadeImpl.groovy

before modify

    @Override
    Calendar getCalendarForTzLc() {
        // the OLD approach using user's TimeZone/Locale, bad idea because user may change for same record, getting different value, etc
        // return efi.getEcfi().getExecutionContext().getUser().getCalendarForTzLcOnly()

        // return Calendar.getInstance(databaseTimeZone, databaseLocale)
        // NOTE: this approach is faster but seems to cause errors with Derby (ERROR 22007: The string representation of a date/time value is out of range)
        // Still causing problems?
        return databaseTzLcCalendar
    }

after modify

    @Override
    Calendar getCalendarForTzLc() {
        // the OLD approach using user's TimeZone/Locale, bad idea because user may change for same record, getting different value, etc
        // return efi.getEcfi().getExecutionContext().getUser().getCalendarForTzLcOnly()

        // return Calendar.getInstance(databaseTimeZone, databaseLocale)
        // NOTE: this approach is faster but seems to cause errors with Derby (ERROR 22007: The string representation of a date/time value is out of range)
        // Still causing problems?
       return Calendar.getInstance(databaseTimeZone, databaseLocale)
    }
jonesde commented 6 years ago

What do you mean by 'date of discovery changed'? Could you describe the issue you observed?

ofbizer commented 6 years ago

When a date field exists in an entity, a data record is inserted in the entity, and when the volume is relatively large, the date of the insertion error occurs, and the wrong date is also irregular. I think the code is a better description of the problem. The date has changed since 'efi.getCalendarForTzLc()' changed the calendar object. I have already printed the calendar object I got and it has changed. I also printed the date parameter, which is always correct. I am sure there is a problem and the performance test is reproducible. You can look at the previous description and I solved the problem. I looked at the source code of the JDBC driver and found that by default I get a new calendar object every time.

All possible issues related to ‘efi.getCalendarForTzLc()’ are problematic

framework/src/main/groovy/org/moqui/impl/entity/FieldInfo.java#setPreparedStatementValue(PreparedStatement ps, int index, Object value, int localTypeValue,boolean useBinaryTypeForBlob, EntityFacadeImpl efi)

code segment

case 4:
    if (value != null) {
        Class valClass = value.getClass();
        if (valClass == java.sql.Date.class) {
            java.sql.Date dt = (java.sql.Date) value;
            // logger.warn("=================== setting date dt=${dt} dt long=${dt.getTime()}, cal=${cal}")
            ps.setDate(index, dt); // Date was likely generated in Java TZ and that's what we want, if DB TZ is different we don't want it to use that
        } else if (valClass == Timestamp.class) {
            ps.setDate(index, new java.sql.Date(((Timestamp) value).getTime()), efi.getCalendarForTzLc());
        } else if (valClass == java.util.Date.class) {
            ps.setDate(index, new java.sql.Date(((java.util.Date) value).getTime()), efi.getCalendarForTzLc());
        } else {
            throw new EntityException("Class " + valClass.getName() + " not allowed for date fields, for field " + entityName + "." + name);
        }
    } else { ps.setNull(index, Types.DATE); }
    break;
jonesde commented 6 years ago

I did some research on this and (not surprisingly) different JDBC drivers do very different things with the Calendar parameter in their PreparedStatement implementations. H2 just gets the TimeZone from the Calendar and otherwise ignores it. MySQL clones the Calendar and modifies it. The Postgres JDBC driver actually keeps a single Calendar object to use as the default and just uses the TimeZone from it. The Oracle JDBC driver must modify and use the actual Calendar object, but I couldn't readily find the source for it.

On a side note there are some weird differences in interpretation of how the TimeZone should be used in MySQL vs Oracle, see: https://bugs.openjdk.java.net/browse/JDK-4986236

This isn't a huge issue but something to keep in mind when setting the TimeZone for your database, ie so that you get date/time values as desired when working directly in the database.

As for a solution I don't want to create a Calendar object each time it is used because from profiling tests this is very slow and I've heard there are internal calls to synchronized static methods that could cause more concurrency issues.

In commit ecdcb6ad I changed it to use a ThreadLocal for one Calendar instance per thread, and also call clear() on the Calendar object to avoid any values set in it from prior use being reused. Without seeing the source for the Oracle JDBC driver I don't know if this is needed, or might even caused problems.

Could you try that commit and see if it works for you?

BTW, there was some recent discussion about the settings to use for the 'oracle' database type but I never heard back on what ended up working. What are the settings you are using (in the Moqui Conf XML file), and are there any other code changes you made?

ofbizer commented 6 years ago

In the production environment, I am using your solution. After verification of the test environment and the operation of the production environment for so many days, the problem is solved. thank u

In the use of oracle, we basically use the default configuration.Occasionally we will customize the entity field type to adapt to the project library table structure design. I also tried to add orale's table and field comment

When there is a problem, or other considerations, I will generally submit it here.