preface

This article is a summary of the problems encountered in the previous development. In fact, the article has been written well, but I feel that I have not written deeply enough, so I let the article lie in the draft box. Yesterday suddenly remembered, will article revise once again, still send out!

Problem 1: Duplicate SQL fields generated by the framework

At that time, I was developing an exception logging interface. In fact, the business process was very simple, that is, the front end submitted the error log to the background, the background received the information for processing, and then inserted into the database. Because the concurrency of this interface is relatively high, in order not to affect other services, and improve the response speed. So use @async annotation + Spring thread pool scheme to achieve. The thread pool configuration is as follows:

<task:annotation-driven executor="jobExecutor"/>
<task:executor id="jobExecutor" pool-size="20" queue-capacity="500" />
Copy the code

Use task: the annotation – driven/when you start the asynchronous, must remember to configure executor attributes, otherwise use asynchronous thread pool is org. Springframework.. The core task. SimpleAsyncTaskExecutor, But this SimpleAsyncTaskExecutor is not a real thread pool. This class does not reuse threads and creates a new thread each time it is called.

Key pseudocodes are as follows:

   @Async
    public void test(a){

        ExceptionLogEntity exceptionLogEntity = new ExceptionLogEntity();
        exceptionLogEntity.setXX("");
        exceptionLogEntity.setXXX("");
        exceptionLogEntity.setIp("");
        exceptionLogEntity.setUrl("");
        exceptionLogEntity.setBusinessScene("");
        exceptionLogEntity.setExceptionType("");
        exceptionLogEntity.setExceptionDetailType("");
        exceptionLogEntity.setExceptionMessage("");
        exceptionLogEntity.setNoticeStatus("");
        exceptionLogEntity.setCreateTime(new Date());
        exceptionLogEntity.setUpdateTime(new Date());


        ExceptionLogEntity insert = exceptionLogDao.insert(exceptionLogEntity);
        log.info("Primary key id=[{}]", insert.getId());

    }
Copy the code

After writing the code, I did a simple test and was told that the front end could be connected without any problems. But a strange thing happened. The front end told me that the interface sometimes returned an error. As soon as I heard it, I felt wrong and thought that such a simple interface, how could I have a bug. So I went to the log platform to query the log, the result is really a problem. An SQL error was found as follows (sensitive information has been handled) :

Caused by: java.lang.reflect.InvocationTargetException

INSERT INTO`xx`(`xx`,`xx`,`ip`,`url`,`business_scene`,`exception_type`,`exception_detail_type`,`exception_message`,`notice_status`, `create_time`,`update_time`,`xx`,`member_phone`,`ip`,`url`,`business_scene`,`exception_type`,`exception_detail_type`,`ex ception_message`,`notice_status`,`create_time`,`update_time`)VALUES(' '.' '.' '.' '.' '.' '.' '.' '.' '.'2020-01-08 19 at sun.reflect.GeneratedMethodAccessor115.invoke(Unknown Source) -------------------------------------------- at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at coderead.mybatis.log.JdbcCommonCollects$PreparedStatementHandler.invoke(JdbcCommonCollects.java:118) ... 21 more Caused by: com.mysql.jdbc.exceptions.jdbc4.MySQLSyntaxErrorException: Column 'update_time' specified twice at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62) at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) at java.lang.reflect.Constructor.newInstance(Constructor.java:423) at com.mysql.jdbc.Util.handleNewInstance(Util.java:425)Copy the code

Recurring problems

The Column ‘update_time’ specified twice. It’s not just the update_time field that’s duplicated, it’s a lot of fields that are duplicated. Because there’s nothing special about this interface other than concurrency! It suddenly occurred to me that the framework was making errors in generating INSERT type SQL because of the high concurrency of the interface. If so, I directly create such a scene can not be realized! Then I wrote the following code:

public void test(a){

        for (int i = 0; i < 1000; i++) {
            ExceptionLogEntity exceptionLogEntity = new ExceptionLogEntity();
            exceptionLogEntity.setXX("");
            exceptionLogEntity.setXXX("");
            exceptionLogEntity.setIp("");
            exceptionLogEntity.setUrl("");
            exceptionLogEntity.setBusinessScene("");
            exceptionLogEntity.setExceptionType("");
            exceptionLogEntity.setExceptionDetailType("");
            exceptionLogEntity.setExceptionMessage("");
            exceptionLogEntity.setNoticeStatus("");
            exceptionLogEntity.setCreateTime(new Date());
            exceptionLogEntity.setUpdateTime(new Date());

            Thread thread = newThread(() -> { exceptionLogDao.insert(exceptionLogEntity); }); thread.start(); }}Copy the code

After the code is written, restart the project directly, and finally trigger the above code. Sure enough, the same mistake happened again. Here I first introduce the ORM framework used:

This ORM framework is the basic component of the GROUP O2O RESEARCH and development center, which provides the same function as Mybatis: just define the interface, no need to write the implementation class. In addition, the single table operation also encapsulated a set of commonly used add, delete, change and check processing, the connection does not need to define, very friendly to developers, greatly reducing the tedious JDBC operation code, improve the development efficiency.

I asked my colleague if he had encountered any problems with duplicate SQL columns when using the INSERT method that comes with the company’s ORM framework. Hey, don’t say it. Everybody said it happened. That’s amazing. It seems that this is really not an isolated phenomenon, but a bug in the framework. So IT took me a while to find out where the problem was, and I found it. The code is as follows:

    public List<DalColumn> getDalColumnsWithoutId(a) {
        if (this.columnsWithoutId ! =null) {
            return this.columnsWithoutId;
        } else {
            this.columnsWithoutId = new ArrayList(this.dalColumns.size() - 1);
            Iterator var1 = this.dalColumns.iterator();

            while(var1.hasNext()) {
                DalColumn column = (DalColumn)var1.next();
                if(! column.isIdColumn()) {this.columnsWithoutId.add(column); }}return this.columnsWithoutId; }}Copy the code

ColumnsWithoutId is of type List. It is an attribute of the class in which columnsWithoutId is null by default. ColumnsWithoutId holds all columns of a table except the primary key ID. The above code is fine in the single-threaded case, but is more problematic in the multi-threaded case. ColumnsWithoutId in the case of multiple threads is a shared variable, so if at some point multiple threads happen to be executing an if block, and columnsWithoutId is null, then else blocks are executed at the same time, and then multiple loops are executed, Naturally, the fields in columnsWithoutId are duplicated.

To solve the problem

Now that the problem has been found, we need a solution.

The official repair

I thought that since the framework was a basic component provided by the company’s R&D center, it meant that many project teams would be using it internally and that the problem would have been fixed long ago. Don’t mention that I actually found an article on the company wiki from the guy responsible for this component called “single table insert accidental column repetition problem location and resolution”. (I will not send the address of the article, and you can not access the internal address of the company.) The article describes in detail the situation where the problem occurred (similar to what I described above), the process of locating the problem and the solution. Abstract Solutions on the article:

Clear the problem is thread-safe, solutions are well identified, solve the thread safety three options: plan a: mutex synchronization – synchronized, already; CAS scheme 2: non-blocking synchronization CAS scheme 3: no synchronization ThreadLocal Based on the service scenario and comprehensive analysis, plan 1 is confirmed to solve the problem by adopting the synchronized keyword and double-checked lock.

The big guys have fixed the bug and released a new version to the Maven repository. If the above problem occurs in the project, you can directly upgrade the corresponding component version. Now that the official solution has been given, I will simply upgrade the component version number in the project. As a result, as soon as I changed the version in the project, my project never started again. The study found that the component versions were too different and the internal structure changed too much. 1.1.6-release was used in my project, and the problem was officially fixed in version 2.1.0-Release. This component version gap is too big rancor upgrade must be a great risk, so I gave up!

Find another way out

Since you have a problem with your own insert method, I’ll just create a custom insert method. I can’t afford to hide! I finally asked my colleagues how to solve the problem in the end, and they all said that custom SQL, and no longer use the framework’s own method, we are very smart! (In fact, this has a follow-up, later in the development process, need to use the batch insert function, I lazy to use the framework of the batch method, and I cheated again. I haven’t used my own methods since…)

Problem 2: Primary key data overflow

When I checked the above problem again, I saw the following code in the framework:

    private Number execute4PrimaryKey(String sqlId, Map<String, Object> paramMap, KeyHolder keyHolder) {
        long startTimestamp = System.currentTimeMillis();
        String sql = null;

        Object var8;
        try {
            MappedStatement mappedStatement = this.configuration.getMappedStatement(sqlId, true);
            mappedStmtThreadLocal.set(mappedStatement);
            sql = mappedStatement.getBoundSql(paramMap);
            int result = false;
            int result;
            if(keyHolder ! =null) {
                this.execution.update(sql, new MapSqlParameterSource(DalUtils.mapIfNull(paramMap)), keyHolder);
                result = keyHolder.getKey() == null ? 0 : keyHolder.getKey().intValue();
            } else {
                result = this.execution.update(sql, DalUtils.mapIfNull(paramMap));
            }

            Integer var9 = result;
            return var9;
        } catch (Exception var13) {
            this.throwException(var13);
            var8 = null;
        } finally {
            mappedStmtThreadLocal.remove();
            logger.debug("{} method:{}, sql:{}, param:{}".new Object[]{this.logPrefix, "execute", sql, paramMap});
            this.logProfileLongTimeRunningSql(startTimestamp, sql, paramMap);
        }

        return (Number)var8;
    }
Copy the code

If you look closely, there’s a problem with this code that’s not easy to spot. Bigint = bigInt; bigInt = bigInt; bigInt = bigInt; bigInt = bigint; Data overflow, by the way.

Recurring problems

I set the primary key self-increment in the table to the maximum value of Java Int +1, which is 2147483648. The primary key ID in the Java entity is already negative — 2147483648.

When using Long, be aware that JavaScript receives backend Long data with lost precision

To solve the problem

This problem is really not a new way, can only later appear similar problems, upgrade the ORM framework version.

Problem 3: Concurrency problem with shared variables

In fact, I should have found this problem when I read the internal article, but I just didn’t see it at the time! At this time, I left tears without technology. For problem 1, synchronized was finally adopted by that boss to solve the problem of reading and writing shared variables in the case of multi-threading. The specific code is as follows (sensitive information coding) :

Code before modification:

public List<DalColumn> getDalColumnsWithoutId(a) {
        if (this.columnsWithoutId ! =null) {
            return this.columnsWithoutId;
        } else {
            this.columnsWithoutId = new ArrayList(this.dalColumns.size() - 1);
            Iterator var1 = this.dalColumns.iterator();

            while(var1.hasNext()) {
                DalColumn column = (DalColumn)var1.next();
                if(! column.isIdColumn()) {this.columnsWithoutId.add(column); }}return this.columnsWithoutId; }}Copy the code

Modified code (refer to the code screenshot in the article) :

Problem of repetition

If this problem is to be repeated, it is not very good to repeat, because the situation is more extreme, but it is likely to occur in a complex production environment.

For example:

Let’s say we have two threads, THREAD A and thread B. Thread A executes at the for loop, and thread B executes at the initial if judgment. Since the cloumnsWithId has been assigned by thread A, it must not be null, and then thread B returns directly. Causes the thread B caller to get an empty or partially fielded cloumnsWithId. The end result of this situation is missing fields in INSERT SQL.

The solution

For multi-threaded shared variables, we must adhere to the principle of parallel reading and queuing reading and writing. Of course, if you can leave it unlocked, it is also a good choice.

After I found this problem, I went to the big man mentioned in the previous article and gave him feedback on my ideas and proposed my solution.

  1. Add synchronized directly to the getDalColumnsWithoutId method.

  2. Modify the first if judgment logic, and then use the synchronized lock block.

    public List<DalColumn> getDalColumnsWithoutId(a) {
        int size = CollectionUtils.size(this.dalColumns);
        if (CollectionUtils.size(this.columnsWithoutId) == size && size > 0 ) {
            return this.columnsWithoutId;
        }
        synchronized(lock){
            if (columnsWithoutId == null) {
                columnsWithoutId = new ArrayList<>(size*2);
            }
            for (DalColumn column :  this.dalColumns){
                if(! column.isIdColumn()) {this.columnsWithoutId.add(column); }}}return this.columnsWithoutId;
    }
Copy the code

Screenshot of communication with the big guy at that time, affirmation from the big guy:

conclusion

Take your time when you have a problem, make bold assumptions, and be careful to verify them. Don’t be afraid to question authority, and don’t assume that because it’s already a framework, there aren’t bugs. For example, Spring has gone from 1.0 to more than 5.0. In addition to adding new features to Spring, other updates are to fix bugs in Spring. After making their own assumptions, it is necessary to perform scene replay, a normal bug can be repeated repeatedly. That is to find the crux of the problem, the rest is how to solve!