Adding bookmarks for cluster support

[tim-hanssen]

It would be really helpful if the connector would start supporting transaction bookmarks. Now we often have to retry queries due to the fact that the data is not their yet. (read after write).

The bookmark is returned after a write commit. In the V1/Session.php run method when the element stats are returned also the bookmark is. It's there in the same elements array.

    if (isset($pullMeta[0])) {

        if (isset($pullMeta[0]->getElements()['stats'])) {

            if(isset($pullMeta[0]->getElements()['bookmark'])) {
                session(['n4jbookmark' => $pullMeta[0]->getElements()['bookmark']]);
            }

            $cypherResult->setStatistics($pullResponse->getMetadata()[0]->getElements()['stats']);
        } else {
            $cypherResult->setStatistics([]);
        }
    }`

Next comes the setting of the bookmark, I tried adding it to the BEGIN message of a transaction as I saw it was done by the javascript bolt connector but when the bookmark property gets filled an exception on the read is thrown.

Any suggestions on how to add the bookmarks to the next transactions?

[matas-valuzis]

As far as I understand (since there are no official documentation) bookmarks in V3+ are sent as metadata and for V1+ as parameters. js driver's run method

 run (
    query,
    parameters,
    {
      bookmark,
      txConfig,
      database,
      mode,
      beforeKeys,
      afterKeys,
      beforeError,
      afterError,
      beforeComplete,
      afterComplete,
      flush = true
    } = {}
  ) {
    const observer = new ResultStreamObserver({
      connection: this._connection,
      beforeKeys,
      afterKeys,
      beforeError,
      afterError,
      beforeComplete,
      afterComplete
    })

    // bookmark and mode are ignored in this version of the protocol
    assertTxConfigIsEmpty(txConfig, this._connection, observer)
    // passing in a database name on this protocol version throws an error
    assertDatabaseIsEmpty(database, this._connection, observer)

    this._connection.write(
      RequestMessage.run(query, parameters),
      observer,
      false
    )
    this._connection.write(RequestMessage.pullAll(), observer, flush)

    return observer
  }

passing bookmarks

beginTransaction ({
    bookmark,
    txConfig,
    database,
    mode,
    beforeError,
    afterError,
    beforeComplete,
    afterComplete
  } = {}) {
    return this.run(
      'BEGIN',
      bookmark ? bookmark.asBeginTransactionParameters() : {}, // passing bookmarks as parameters
      {
        bookmark: bookmark,
        txConfig: txConfig,
        database,
        mode,
        beforeError,
        afterError,
        beforeComplete,
        afterComplete,
        flush: false
      }
    )
  }

It would be helpful if you could add a test case for bookmarks (as I understand they are used only in enterprise?) so it would be easier to test implementation for various versions of the protocol.

[tim-hanssen]

Hi @matas-valuzis it's hard to make a test since the bolt responses doesn't seem to throw any suggestion in of the bookmarks are accepted or not. I think its enterprise only since it only applies to Neo4j clusters. But without it, read after writes are very un-predictive.