Skip to content

Conversation

@aakashanandg
Copy link
Collaborator

feat: Adding support for exclude_txn_from_change_streams option while creating RW transactions.

This PR introduces support for the exclude_txn_from_change_streams option for RW transactions. This allows developers to selectively prevent specific transactions from being recorded in a database's change stream.

Usage Example:

# This transaction will NOT be captured by any change stream.
Singer.transaction(exclude_txn_from_change_streams: true) do
   # Perform high-volume or low-value writes here.
  Singer.upsert_all(...)
end

Fixes #367

@aakashanandg aakashanandg requested review from a team and olavloite as code owners June 11, 2025 18:18
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/ruby-spanner-activerecord API. label Jun 11, 2025
olavloite
olavloite previously approved these changes Jun 12, 2025
Copy link
Collaborator

@olavloite olavloite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with a few minor nits


class Session
def commit_transaction transaction, mutations = [], commit_options: nil
def commit_transaction transaction, mutations = [], commit_options: nil, exclude_txn_from_change_streams: false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I don't think we need this in the commit request. This only goes in the BeginTransaction request.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took reference from Ruby client library where this attribute was getting set during the commit call. For ref:https://github.com/googleapis/ruby-spanner/blob/a2e5c8c8b8a909746bbc9aff1b79884ed44db104/google-cloud-spanner/lib/google/cloud/spanner/session.rb#L656

But anyways i will remove this from this commit as it is not required.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yeah, I understand that. The difference here is that:

  1. A CommitRequest can also start a transaction. This is called a single-use write-only transaction, and in such a case the CommitRequest both starts and commits the transaction. This is sometimes used for bulk writing. The drawback of these transactions is that there is no guarantee that they will be applied only once. It could be that a component somewhere in the network path between the client and the server replays the request, for example due to a temporary network problem leading to an UNAVAILABLE error, and in that case the transaction would be applied twice.
  2. Ruby ActiveRecord always starts the transaction first, meaning that the CommitRequest will never start a transaction. So in this case we never need this option here.

assert_equal 1000, commit_options[:max_commit_delay]
end

def test_exclude_txn_from_change_streams
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test does not really appear to test anything relevant. The exclude_txn_from_change_streams flag goes into the BeginTransaction options, but this test sets that flag after the transaction has started, and dos not verify anything regarding the BeginTransaction options.

Copy link
Collaborator Author

@aakashanandg aakashanandg Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. At first, I believed this attribute had to be set during the commit call. I've now updated the test to assert the TransactionSelector object assigned during the transaction.begin call instead.


def test_upsert_all_dml_with_exclude_from_change_streams
sql = "INSERT OR UPDATE INTO `singers` (`id`,`first_name`,`last_name`) " +
"VALUES (1, 'Dave', 'Allison'), (2, 'Alice', 'Davidson'), (3, 'Rene', 'Henderson')"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This SQL statement would in the real world not return 3 rows, it would only return an update count. It would slightly improve the readability of this test to add THEN RETURN * to the SQL string, so it matches what the mock return value is.

@aakashanandg aakashanandg merged commit 1b04c70 into googleapis:main Jun 12, 2025
41 checks passed
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/ruby-spanner-activerecord API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for exclude_txn_from_change_streams

2 participants