0

We're having an issue in our main data synchronization back-end function. Our client's mobile device is pushing changes daily, however last week they warned us some changes weren't updated in the main web app.

After some investigation in the logs, we found that there is indeed a single transaction that fails and rollback. However it appears that all the transactions before this one also rollback.

The code works this way. The data to synchronize is an array of "changesets", and each changset can update multiple tables at once. It's important that a changset be updated completely or not at all, so each is wrapped in a transaction. Then each transaction is executed one after the other. If a transaction fails, the others shouldn't be affected.

I suspect that all the transactions are actually combined somehow, possibly through the main db.task. Instead of just looping to execute the transactions, we're using a db.task to execute them in batch avoid update conflicts on the same tables.

Any advice how we could execute these transactions in batch and avoid this rollback issue?

Thanks, here's a snippet of the synchronization code:

// Begin task that will execute transactions one after the other

db.task(task => {

const transactions = [];

// Create a transaction for each changeset (propriete/fosse/inspection)
Object.values(data).forEach((change, index) => {
  const logchange = { tx: index };
  const c = {...change}; // Use a clone of the original change object

  transactions.push(
    task.tx(t => {
      const queries = [];

      // Propriete
      if (Object.keys(c.propriete.params).length) {
        const params = proprietes.parse(c.propriete.params);
        const propriete = Object.assign({ idpropriete: c.propriete.id }, params);
        logchange.propriete = { idpropriete: propriete.idpropriete };
        queries.push(t.one(`SELECT ${Object.keys(params).join()} FROM propriete WHERE idpropriete = $1`, propriete.idpropriete).then(previous => {
          logchange.propriete.previous = previous;
          return t.result('UPDATE propriete SET' + qutil.setequal(params) + 'WHERE idpropriete = ${idpropriete}', propriete).then(result => {
            logchange.propriete.new = params;
          })
        }));
      }
      else delete c.propriete;

      // Fosse
      if (Object.keys(c.fosse.params).length) {
        const params = fosses.parse(c.fosse.params);
        const fosse = Object.assign({ idfosse: c.fosse.id }, params);
        logchange.fosse = { idfosse: fosse.idfosse };
        queries.push(t.one(`SELECT ${Object.keys(params).join()} FROM fosse WHERE idfosse = $1`, fosse.idfosse).then(previous => {
          logchange.fosse.previous = previous;
          return t.result('UPDATE fosse SET' + qutil.setequal(params) + 'WHERE idfosse = ${idfosse}', fosse).then(result => {
            logchange.fosse.new = params;
          })
        }));
      }
      else delete c.fosse;

      // Inspection (rendezvous)
      if (Object.keys(c.inspection.params).length) {
        const params = rendezvous.parse(c.inspection.params);
        const inspection = Object.assign({ idvisite: c.inspection.id }, params);
        logchange.rendezvous = { idvisite: inspection.idvisite };
        queries.push(t.one(`SELECT ${Object.keys(params).join()} FROM rendezvous WHERE idvisite = $1`, inspection.idvisite).then(previous => {
          logchange.rendezvous.previous = previous;
          return t.result('UPDATE rendezvous SET' + qutil.setequal(params) + 'WHERE idvisite = ${idvisite}', inspection).then(result => {
            logchange.rendezvous.new = params;
          })
        }));
      }
      else delete change.inspection;

      // Cheminees
      c.cheminees = Object.values(c.cheminees).filter(cheminee => Object.keys(cheminee.params).length);
      if (c.cheminees.length) {
        logchange.cheminees = [];
        c.cheminees.forEach(cheminee => {
          const params = cheminees.parse(cheminee.params);
          const ch = Object.assign({ idcheminee: cheminee.id }, params);
          const logcheminee = { idcheminee: ch.idcheminee };
          queries.push(t.one(`SELECT ${Object.keys(params).join()} FROM cheminee WHERE idcheminee = $1`, ch.idcheminee).then(previous => {
            logcheminee.previous = previous;
            return t.result('UPDATE cheminee SET' + qutil.setequal(params) + 'WHERE idcheminee = ${idcheminee}', ch).then(result => {
              logcheminee.new = params;
              logchange.cheminees.push(logcheminee);
            })
          }));
        });
      }
      else delete c.cheminees;

      // Lock from further changes on the mobile device
      // Note: this change will be sent back to the mobile in part 2 of the synchronization
      queries.push(t.result('UPDATE rendezvous SET timesync = now() WHERE idvisite = $1', [c.idvisite]));

      console.log(`transaction#${++transactionCount}`);

      return t.batch(queries).then(result => { // Transaction complete
        logdata.transactions.push(logchange);
      });
    })
    .catch(function (err) { // Transaction failed for this changeset, rollback
        logdata.errors.push({ error: err, change: change }); // Provide error message and original change object to mobile device
        console.error(JSON.stringify(logdata.errors));
    })
  );
});

console.log(`Total transactions: ${transactions.length}`);

return task.batch(transactions).then(result => { // All transactions complete
  // Log everything that was uploaded from the mobile device
  log.log(res, JSON.stringify(logdata));
});
greenkarmic
  • 429
  • 4
  • 17

1 Answers1

0

I apologize, this is almost impossible to make a final good answer when the question is wrong on too many levels...

It's important that a change set be updated completely or not at all, so each is wrapped in a transaction.

If the change set requires data integrity, the whole thing must be one transaction, and not a set of transactions.

Then each transaction is executed one after the other. If a transaction fails, the others shouldn't be affected.

Again, data integrity is what a single transaction guarantees, you need to make it into one transaction, not multiple.

I suspect that all the transactions are actually combined somehow, possibly through the main db.task.

They are combined, and not through task, but through method tx.

Any advice how we could execute these transactions in batch and avoid this rollback issue?

By joining them into a single transaction.

You would use a single tx call at the top, and that's it, no tasks needed there. And in case the code underneath makes use of its own transactions, you can update it to allow conditional transactions.

Also, when building complex transactions, an app benefits a lot from using the repository patterns shown in pg-promise-demo. You can have methods inside repositories that support conditional transactions.

And you should redo your code to avoid horrible things it does, like manual query formatting. For example, never use things like SELECT ${Object.keys(params).join()}, that's a recipe for disaster. Use the proper query formatting that pg-promise gives you, like SQL Names in this case.

vitaly-t
  • 24,279
  • 15
  • 116
  • 138
  • Hi Vitaly-t, there is definitely space for improvement, I won't argue with you there. I expected this type response when I wrote my question. You misunderstood a bit, in that only individual changeset need to be data integral and so each are in their own transaction, i.e. if one fails it doesn't affect the others. The idea is that if one fails, the rest of the data synchronization succeeds. – greenkarmic Jul 23 '19 at 13:37
  • However, your answer made me think that maybe this premise in itself is wrong, a bad workaround to save ourselves from minor synchronization issues, and that the whole thing (all changesets) should be integral. This would mean that if a single one fails, the whole synchronization fails. Drastic (especially when it happens in production and the customer can't synchronize anything) , but it would guarantee integrity as a whole. I will speak with management about this. Thanks – greenkarmic Jul 23 '19 at 13:37