Transaction does not support with merge and have unexpected promise error out of the process

[terence410]

Below codes will prompt " UnhandledPromiseRejectionWarning: Error: 10 ABORTED: too much contention on these datastore entities. please try again." without able to be caught in any try catch block.

If I use transaction.save() or transaction.update(), things work normally.

===============

async function merge(taskKey: any) {
    const transaction = datastore.transaction();
    try {
        await transaction.run();
        const [myTask] = await transaction.get(taskKey);
        const newTask = {
            key: taskKey,
            data: {
                value: 1,
            },
        };
        transaction.merge(newTask);
        await transaction.commit();
    } catch (err) {
        console.log("error", err.message);
        await transaction.rollback();
        throw err;
    }
}

async function test3() {
    // The Cloud Datastore key for the new entity
    const kind = "transaction";
    const taskKey = datastore.key([kind, 1]);

    // Prepares the new entity
    const task = {
        key: taskKey,
        data: {
            value: 10,
        },
    };
    await datastore.save(task);

    // Saves the entity
    try {
        const promises = [
            merge(taskKey),
            merge(taskKey),
        ];
        await Promise.all(promises);
    } catch (err) {
        console.log("error", err.message);
    }
}

[terence410]

things work for merge, just the transaction conflict error will go out somewhere without being able to be caught.

[bcoe]

@AVaksman any thoughts on this one?

[laljikanjareeya]

@terence410 you are getting UnhandledPromiseRejectionWarning because you have not written await before the merge statement: it would be like:

await transaction.merge(newTask);

Now I am getting below error:

error 10 ABORTED: too much contention on these datastore entities. please try again. entity groups: [(app=s~project-id, transaction, 1)]
547.ts:18
error 10 ABORTED: too much contention on these datastore entities. please try again. entity groups: [(app=s~project-id, transaction, 1)]

Merge has different functionality than the save and update.

internally it fetch the entity and merge the both the entity that you provided and one that got from server.

you don't need extra call to get entity in merge as it does already.

could you try this in your case

    const tasks = [{
        key: 'taskKey1',
        data: {
            value: 10,
        }
    }, {
        key: 'taskKey2',
        data: {
            value: 11,
        }
    }]
    await datastore.merge(tasks);

merge is already in transaction you don't need any transaction. please refer below sample for merge: https://github.com/googleapis/nodejs-datastore/blob/7d111c24c6043ea07f91b27a3a6846ed53977fac/samples/tasks.js#L121

[terence410]

I am using @google-cloud/datastore, 5.0.3 for below test cases.

Refer to your reply above, do you mean that merge shouldn't be used with transaction? If yes, will you remove the transaction.merge functions? I would love to see if I can use merge in transaction.

Nevertheless, the below example illustrate what's going on. In short, the merge is broken when using with transaction.

There are some problems:

• I can't do await transaction.merge, this will hang up
• If i dont' use await (which is my expectation on using transaction, because it should submit things by batch). Afterward, there are consistency problem. I guess it's because the transaction won't await for the merge to complete before the commit ends
• If transaction.merge raise any conflict error, I am still not able to capture it by try catch, I believe the merge action in transaction missed a await internally to cause all above problems.

// "@google-cloud/datastore": "^5.0.3"
import * as Datastore from "@google-cloud/datastore";
const datastore = new Datastore.Datastore({keyFilename: "./datastoreServiceAccount.json"});

async function transactionTest1() {
    const kind = "transaction";
    const taskKey = datastore.key([kind, 1]);

    const task = {
        key: taskKey,
        data: {value: 1},
    };
    await datastore.save(task);

    // we use transaction to do save
    const transaction = datastore.transaction();
    await transaction.run();
    const [myTask] = await transaction.get(taskKey);
    const newTask = {
        key: taskKey,
        data: {
            value: 999,
        },
    };
    transaction.merge(newTask);
    // await transaction.merge(newTask);
    // I can't use await since the process will hang up forever
    await transaction.commit();

    // it has consistency problem, the value is 1 instead of 99
    const [result1] = await datastore.get(taskKey);
    console.log(result1);

    // after waiting for a while, the value become 99
    await new Promise(resolve => setTimeout(resolve, 500));
    const [result2] = await datastore.get(taskKey);
    console.log(result2);
}

transactionTest1();

[terence410]

by the way, I have implemented a orm package using datstore, it will be great if you can give some comments https://www.npmjs.com/package/ts-datastore-orm

[meredithslota]

The linked PR https://github.com/googleapis/nodejs-datastore/pull/694 was never merged, due to it being a breaking change. But a comment mentioned we could reconsider when we dropped Node 10: https://github.com/googleapis/nodejs-datastore/pull/694#issuecomment-832942960 It looks like we did that just recently (https://github.com/googleapis/nodejs-datastore/releases/tag/v7.0.0).

Related issue (tagged as a question) has this comment from @vicb:

That is the Transaction section never mentions that Transaction inherits from DatastoreRequest. You have to look at the code examples to know that you can call transaction.get().

It would be great that the get method is documented on transactions or at the very least that Transaction inherits from DatastoreRequest.

Closing the question as a duplicate since this issue has more context.

[meredithslota]

This PR seems to have had the most scrutiny, in case @danieljbruce wants to dust it off in future iterations: https://github.com/googleapis/nodejs-datastore/pull/694

[danieljbruce]

Just looking quickly, seems @laljikanjareeya has good suggestions.

Based on I can't do await transaction.merge, this will hang up, it looks like this is a separate issue altogether.

[danieljbruce]

I agree with @laljikanjareeya. Instead of having multiple promises that each do a merge I suggest doing one merge that includes all the entities you want to change. Note that in the code sample provided, it works if you either add await as suggested or just do one merge.

When doing multiple merges at the same time on the same key, which order were you expecting them to happen in? I suspect this error is expected behaviour because the code is trying to do multiple merges at the same time. The final result is dependent on the order the merges are done so when order is ambiguous as is the case here then it makes sense to throw an error.

[danieljbruce]

More thoughts on the linked PR in case we really do want to avoid the workaround.

[danieljbruce]

Lowering priority to P3 as there is a pending feature to support this.

[danieljbruce]

In the original script new transactions are created both in the merge function and inside each transaction.merge call. Multiple transactions trying to make changes at the same time on the same data will result in the error you are seeing. Modifying the script slightly to use one transaction object throughout will allow the script to work:

// Imports the Google Cloud client library
const {Datastore} = require('@google-cloud/datastore');

// Creates a client
const datastore = new Datastore();
const DatastoreRequest = datastore.DatastoreRequest;
const arrify = require('arrify');

// Changes to original script:
// 1. Replace the transaction.merge call with a call to doMerge.
// 2. Move transaction creation and transaction.commit outside of merge function.

// This function contains the internals of transaction.merge with only 3 changes:
// 1. It doesn't instantiate a new transaction
// 2. It doesn't call transaction.run to begin the transaction
// 3. It doesn't commit the transaction
// In the future we can think about having transaction.merge(entities)
// do the same thing as calling doMerge(transaction, entities) so that
// this is built into the SDK.
async function doMerge(transaction, entities) {
  try {
    await Promise.all(
        arrify(entities).map(async (objEntity) => {
          const obj =
              DatastoreRequest.prepareEntityObject_(objEntity);
          const [data] = await transaction.get(obj.key);
          obj.method = 'upsert';
          obj.data = Object.assign({}, data, obj.data);
          transaction.save(obj);
        })
    );
  } catch (err) {
    try {
      await transaction.rollback();
    } catch (error) {
      // Provide the error & API response from the failed commit to the user.
      // Even a failed rollback should be transparent.
      // RE: https://github.com/GoogleCloudPlatform/gcloud-node/pull/1369#discussion_r66833976
    }
    throw err;
  }
}
async function merge(transaction, taskKey) {
  try {
    const [myTask] = await transaction.get(taskKey);
    const newTask = {
      key: taskKey,
      data: {
        value: 1,
      },
    };
    await doMerge(transaction, newTask);
  } catch (err) {
    console.log("error", err.message);
    await transaction.rollback();
    throw err;
  }
}

async function test3() {
  // The Cloud Datastore key for the new entity
  const kind = "transaction";
  const taskKey = datastore.key([kind, 1]);

  // Prepares the new entity
  const task = {
    key: taskKey,
    data: {
      value: 10,
    },
  };
  await datastore.save(task);

  // Saves the entity
  try {
    const transaction = datastore.transaction();
    await transaction.run();
    const promises = [
      merge(transaction, taskKey),
      merge(transaction, taskKey),
    ];
    await Promise.all(promises);
    await transaction.commit();
  } catch (err) {
    console.log("error", err.message);
  }
}

test3();