v4 doesn't release resource

[ert78gb]

Thanks for stopping by to let us know something could be better!

The library does not release a resource and the unit tests don't exist.
Is there a new command that we have to call before exiting from the application?

Environment details

• OS: Mac, Linux, Windows
• Node.js version: latest of 8, 10, 11
• npm version: provided by node
• @google-cloud/datastore version: 4.0.0

Steps to reproduce

1. $ docker pull google/cloud-sdk:latest
2. $ git clone https://github.com/ert78gb/google-datastore-emulator.git
3. $ cd google-datastore-emulator
4. $ npm i
5. $ npm install --no-save @google-cloud/datastore
6. $ npm test

All test is pass, but mocha does not exists. The reason is there is a resource, timer, promise that running in the background and mocha waiting for the exit.

The 3.1.2 version of the lib worked perfectly, but something changed. You can see it in the travis build logs https://travis-ci.org/ert78gb/google-datastore-emulator/builds

I don't want to use process.exit() at the end of the test, because it hides resource release problems and it is also not a good practice.

Making sure to follow these steps will guarantee the quickest resolution possible.

Thanks!

[laljikanjareeya]

I tried with both the version of @google-cloud/datastore, but the behavior is same in both version. Please confirm, it is working with @google-cloud/datastore 3.1.2 version.

if not than you have to mention explicitly --exit in mocha command as default behavior of mocha has been changed since mocha 4.x version.

Please refer this for more information.

I tried downgrading the mocha to 3.x in above given repository and it's working fine.

[ert78gb]

I will create a test matrix on the weekend that uses the latest version of 3 and 4 of the datastore

Last time when I tried on my Mac pro the 3.x exited correctly

[ert78gb]

In the logs, you can see v3 works fine v4 hanging

[laljikanjareeya]

@ert78gb try using --exit flag while executing tests via mocha.

[ert78gb]

Never, it is hide the resource management problems. It was the reason why changed the default behavior of mocha and I agree with it

[stephenplusplus]

@ert78gb I appreciate the reproduction case, but is it possible to narrow it down to the smallest possible scenario? This makes the debugging process a lot more straightforward, since in this case I would be able to bypass the custom classes and Docker integration.

[ert78gb]

I will create a very simple example tomorrow.
The script will

• connect to the bucket
• and add a document

The script will not exist.
I am experiencing the same problem with the latest pupsub lib. I think one of the common components causes the problem.
My guess is there is a timer to keep alive the connection or renew a token periodically.

[stephenplusplus]

Thank you very much for your help!

[ert78gb]

sure. I also would like to know what is the root cause :)

[ert78gb]

@stephenplusplus Sorry for the late answer. But was very hard to reproduce the problem.

When I created the sample repo I can not reproduce the bug with the v4 of Datastore, so I tried to investigate what is the problem.

What happens in the datastore-emulator tests:

1. Start the Google Datastore emulator - my tool
2. Read/Write operation - Datastore client
3. Stop the Google Datastore emulator - my tool
4. mocha does not exist.

Mocha exists only when no pending event in the event loop.
Let's see a little bit deeper the process

1. Start the Google Datastore emulator - my tool
2. Read/Write operation - Datastore client
3. Stop the Google Datastore emulator - my tool
   3.1 Google Datastore emulator terminating
   3.2 Http2Channel of the grcp-js detects the connection breakings and starts a timer for the auto reconnection. It will an infinity loop
4. mocha does not exist.

The Http2Channel of the grcp-js has a close method. If I see well it stops the timer that blocks the exits of the event loop.

It is possible to add a close() method to the Datastore client that call Http2Channel.close()?
Or maybe add an auto-reconnect property to the Http2Channel. If the property true then the auto-reconnect supported if false then not.
Or ...
I don't know which solution is match better to Google coding style.

[ert78gb]

@stephenplusplus What do you think about my prev comment?

[stephenplusplus]

@ert78gb sorry for the delay. I have been working on this, but I'm still hoping you could present the issue in a way that's detached from the wrapper you've written. I've spent a lot of time trying to work within it and make sure any holes that might exist are plugged, but there are too many variables to be able to track it down with confidence. As a start, maybe you could assume a Docker instance is already running and remove the standing up and tearing down logic? Ideally, something so small and using only our library that it leaves only us to blame 😆

[ert78gb]

The small repo that contains only the datastore emulator code
https://github.com/ert78gb/google-datastore-client-not-exits

[stephenplusplus]

Thanks! The idea of implementing a close() method makes sense, although this could need a more thorough review.

@JustinBeckwith @bcoe @callmehiphop @AVaksman -- grpc-js opens a channel with a server, then if the connection is interrupted, it runs an interval to check back and see if the server comes online again. This can be reproduced by stopping the Datastore emulator after a single call, datastore.save(), is made but before the script naturally exits. The script will fail to exit due to grpc-js's optimism that the server will come to life again. If you start it up again, you'll see that the script successfully exits.

So, this works:

• Run script which calls datastore.save()
• grpc-js:
  • opens connection
  • makes request
  • gets response
  • returns response
• 👍 Script exits

And this works:

• Run script which calls datastore.save()
• grpc-js:
  • opens connection
  • makes request
  • gets response
  • returns response
• Stop emulator
• Re-start emulator
• 👍 Script exits

But this doesn't:

• Run script which calls datastore.save()
• grpc-js:
  • opens connection
  • makes request
  • gets response
  • returns response
• Stop emulator
• 👎 Script won't exit

So, it seems like after we get the response from calling datastore.save(), grpc-js is keeping the channel open, even though we might not use it again. Otherwise, it wouldn't be surprised by connection breaking when the emulator stops. Is anyone more familiar with how this process is supposed to work, and what a recommended approach might be for handling this?

[stephenplusplus]

I believe I have found a solution to expose the channel's close method. PR coming shortly!

[ert78gb]

thx. I am waiting.

[stephenplusplus]

Sorry for the lack of an update. Turns out there’s a bug with grpc that has an open PR for a fix: grpc/grpc-node#1015. When that goes, the original behavior will be restored, and the script should exit as expected.

[bcoe]

@stephenplusplus it looks like the upstream work for grpc is merged, and a new library version was recently published.

@ert78gb can you confirm that things are working?

[ert78gb]

With the @grpc/[email protected] works perfectly.
Can you upgrade the dependency and release a new version?
By default, the npm i does not install the latest version of it.

[bcoe]

@ert78gb 👍 awesome, I'm glad this has addressed your issue. We will work on explicitly updating the version # of grpc this week. In the mean time, I think you should get the desired effect by rm -rf package-lock.json; rm -rf node_modules, in a project.

I'm going to go ahead and close this issue since the underlying issue is addressed. Let me know if you bump into any more issues.