Uploaded image for project: 'Node.js Driver'
  1. Node.js Driver
  2. NODE-6633

Track active cursors on MongoClient and close them along with the client

    • Type: Icon: Task Task
    • Resolution: Unresolved
    • Priority: Icon: Major - P3 Major - P3
    • None
    • Affects Version/s: None
    • Component/s: None
    • Not Needed

      Use Case

      As a... Node driver user
      I want... the lifetime of the cursor to be tied to the MongoClient that owns it
      So that... when the client is closed so are the related cursors avoiding leaked resources

      User Experience

      • What is the desired/expected outcome for the user once this ticket is implemented?
        • When MongoClient.close is called, every active cursor has its close method called.
        • Any pending promise returned from the cursor is rejected as a result (ex. toArray() or for await usage)

      Dependencies

      • Potentially needs NODE-6632 first to reject cursor operations

      Risks/Unknowns

      • What could go wrong while implementing this change? (e.g., performance, inadvertent behavioral changes in adjacent functionality, existing tech debt, etc)
        • Great care should be taken to ensure the client stops maintaining a reference to a cursor when it is closed.
        • Cursors already threw an error when the client was closed. It was typically "cannot use ended session" when an automatically executed getMore would be unable to use the session (since the MongoClient already tracks and ends all sessions). However, this also could have been a pool closed error, or a server selection error, given the right timing.
          • Since it will now be the AbortController closing the active socket / closing the pool / stopping server selection we should be able to assert the same meaningful error is always thrown from these methods. This change should be tested and called out in the notes.
      • Is there an opportunity for better cross-driver alignment or testing in this area?
        • No
      • Is there an opportunity to improve existing documentation on this subject?
        • No

      Acceptance Criteria

      Implementation Requirements

      • Whenever an cursor is created on the server track it in a set of activeCursors on the client and remove it whenever that cursor is closed.
        • Note: rewind() should not remove it from being active
      • A MongoClient's close method will close all the active cursors
        • Some perf improvements that we can attempt or file followups:
          • Attempt to group the ids by server and send killCursors with an array of ids to each server
          • Parallelize the close calls with Promise.all
          • Fire and forget the command(s) using OP_MSG protocol

      Testing Requirements

      • Check that cursors are added and removed from the active set regardless of un/happy scenarios
        • Errors generally always call cursor.close, we should orchestrate some server error to test this
        • Happy path cursor usage that implicitly cleans up the cursor should also remove from the set
        • Explicit close calls should remove it from the set
      • Test MongoClient close does close all cursors in the set
        • Check we sent the expected commands
          • if we end up sending one per cursor that's fine but we'll want this test to help us check our future optimizations if we can merge the ids
        • Check each cursor's close method was called - a very specific unit test that maybe we don't need but if this is the expected "lifetime end" function I would want to know if the client switches to a new one.

      Documentation Requirements

      • Update MongoClient.close documentation to describe what happens to cursors

      Follow Up Requirements

      • Potential improvements mentioned in AC

            Assignee:
            neal.beeken@mongodb.com Neal Beeken
            Reporter:
            neal.beeken@mongodb.com Neal Beeken
            Aditi Khare
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: