Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep creating new sessions before max is reached, not using existing ones. #750

Closed
hengfengli opened this issue Dec 15, 2019 · 1 comment
Assignees
Labels
api: spanner Issues related to the googleapis/nodejs-spanner API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@hengfengli
Copy link
Contributor

Environment details

  • OS:macOS 10.14.6
  • Node.js version:v13.3.0
  • npm version:6.13.2
  • @google-cloud/spanner version:4.4.0

Steps to reproduce

index.ts:

/*
Table schema: 

CREATE TABLE Users (
    Name STRING(MAX),
    MySkills ARRAY<STRING(MAX)>,
    NeededSkills ARRAY<STRING(MAX)>,
    OfficeLocations ARRAY<STRING(MAX)>,
    PreferredDays STRING(MAX),
) PRIMARY KEY (Name)
*/
const { Spanner } = require('@google-cloud/spanner');

function readWriteTransaction(index, database) {
  return new Promise((resolve, reject) => {
    database.runTransaction(async (err, transaction) => {
      console.log("running i: ", index);

      if (err) {
        console.error(err);
        return;
      }
      const queryOne = {
        columns: ['Name', 'NeededSkills'],
        keySet: {
          all: true,
        },
      };
      const queryTwo = {
        columns: ['Name', 'MySkills'],
        keySet: {
          all: true,
        },
      };
  
      Promise.all([
        transaction.read('Users', queryOne).then(results => {
          const rows = results[0].map(row => row.toJSON());
          console.log(`NeededSkills: ${rows[0].NeededSkills}`);
        }),
  
        transaction.read('Users', queryTwo).then(results => {
          const rows = results[0].map(row => row.toJSON());
          console.log(`MySkills: ${rows[0].MySkills}`);
        }),
      ])
        .then(() => {
          // Updates the database
          // Note: Cloud Spanner interprets Node.js numbers as FLOAT64s, so they
          // must be converted (back) to strings before being inserted as INT64s.
          transaction.update('Users', [
            {
              Name: 'mdbuser/hengfeng',
              MySkills: ["test", "program"],
            },
          ]);
        })
        .then(() => {
          // Commits the transaction and send the changes to the database
          return transaction.commit();
        })
        .then(() => {
          console.log(
            `Successfully executed read-write transaction.`
          );
        })
        .catch(err => {
          console.error('ERROR:', err);
          reject(err)
        })
        .then(() => {
          transaction.end();  // cause an issue if not calling it, when an error happens. 
  
          console.log("display session pool:")
          database.display();
          resolve()

          
          // return database.close();
        });
    });
  });
};

const projectId = "e2e-debugging";
const instanceId = "hengfeng-test-instance";
const databaseId = "test-db";

async function runTest() {
  // Creates a client
  const spanner = new Spanner({
    projectId: projectId,
  });

  // Gets a reference to a Cloud Spanner instance and database
  const instance = spanner.instance(instanceId);
  const database = instance.database(databaseId, {
    max: 100,
    concurrency: 5,
    writes: 0.1,
    // fail: true,
  });

  const promises = [];

  for (var _i = 0; _i < 100; _i++) {
    promises.push(readWriteTransaction(_i, database));
  }
  
  await Promise.all(promises);

  console.log("final display:");
  database.display();

  // Closes the database when finished
  console.log("closing the database connection.");
  database.close();
}

runTest();

Added a display() method in database.ts:

    display() {
        this.pool_.display()
    }

Added a display() method in session-pool.ts:

    display() {
        console.log("readonly:", this._inventory["readonly" /* ReadOnly */].length);
        console.log("readwrite:", this._inventory["readwrite" /* ReadWrite */].length);
        console.log("borrowed:", this._inventory.borrowed.size);
        console.log("_traces:", this._traces.size);
    }

Expected behaviour

It should reuse existing sessions instead of keeping creating new sessions.

Actual behaviour

If you run the code, you will get:

...
final display:
readonly: 0
readwrite: 99
borrowed: 1
_traces: 1

It created 100 sessions.

@hengfengli hengfengli added the type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. label Dec 15, 2019
@olavloite olavloite self-assigned this Dec 16, 2019
@olavloite
Copy link
Contributor

This behavior is as expected, as the code example you provided is executing multiple read/write transactions in parallel. The readWriteTransaction method in your example will start executing a read/write transaction and return a Promise for its result. Pushing those promises to a list and then waiting for all of them to finish will allow the transactions to execute in parallel. The exact number of transactions that execute in parallel will depend on the execution time of the transactions, but in general they will all be started in parallel as the RPCs will most probably be slower than the client side code for checking out a session and starting a transaction.

If you were to run the transactions sequentially, the session pool will only contain one session after executing all the transactions.

See also

it('should reuse write sessions', async () => {
in PR #743

@google-cloud-label-sync google-cloud-label-sync bot added the api: spanner Issues related to the googleapis/nodejs-spanner API. label Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/nodejs-spanner API. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

2 participants