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

Wrong transaction as current transaction in datastore callbacks #14

Open
step76 opened this issue Jun 20, 2022 · 2 comments
Open

Wrong transaction as current transaction in datastore callbacks #14

step76 opened this issue Jun 20, 2022 · 2 comments
Assignees

Comments

@step76
Copy link

step76 commented Jun 20, 2022

Hello,

I'm using the datastore callbacks to log the entity groups involved in a single transaction. The callbacks fire correctly but I've noticed in the logs some elements for which I explicitly passed null as a transaction.

So, digging the code I've found that the context passed to datastore callbacks is not populated with the transaction received in the get/put/delete/prepare methods, but is taken from an outer context.

  @Override
  public Future<Map<Key, Entity>> get(@Nullable Transaction txn, Iterable<Key> keys) {
    if (keys == null) {
      throw new NullPointerException("keys cannot be null");
    }

    // TODO: The handling of the Keys is pretty ugly.  We get an Iterable from the
    // user.  We need to copy this to a random access list (that we can mutate) for the PreGet
    // stuff. We will also convert it to a HashSet to support contains checks (for the RemoteApi
    // workaround).  There are also some O(N) calls to remove Keys from a List.
    List<Key> keyList = Lists.newArrayList(keys);

    // Allocate the Map that will receive the result of the RPC here so that PreGet callbacks can
    // add results.
    Map<Key, Entity> resultMap = new HashMap<Key, Entity>();
    PreGetContext preGetContext = new PreGetContext(this, keyList, resultMap);
    datastoreServiceConfig.getDatastoreCallbacks().executePreGetCallbacks(preGetContext);

    // Don't fetch anything from datastore that was provided by the preGet hooks.
    keyList.removeAll(resultMap.keySet());

    // Send the RPC(s).
    Future<Map<Key, Entity>> result = doBatchGet(txn, Sets.newLinkedHashSet(keyList), resultMap);

    // Invoke the user post-get callbacks.
    return new PostLoadFuture(result, datastoreServiceConfig.getDatastoreCallbacks(), this);
  }

For example, in the get method the context is created using this as transaction provider whilst the transaction is explicitly passed in the method, this lead to a wrong current transaction inside the callback.

Thank you
Stefano

@ludoch ludoch self-assigned this Aug 8, 2022
@ludoch
Copy link
Collaborator

ludoch commented Aug 8, 2022

Asking @dgay42

@ludoch
Copy link
Collaborator

ludoch commented May 9, 2023

Filed internal issue b/281725939

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants