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

[Actor] reentrancy setting not works #365

Open
raymondsze opened this issue Sep 18, 2022 · 7 comments
Open

[Actor] reentrancy setting not works #365

raymondsze opened this issue Sep 18, 2022 · 7 comments
Assignees
Labels
bug Something isn't working triaged/resolved

Comments

@raymondsze
Copy link

raymondsze commented Sep 18, 2022

Expected Behavior

step2 should be logged as reentrancy is enabled.

Actual Behavior

step2 is not logged because of the same actorId locking.

Steps to Reproduce the Problem

This is my code. If I understand it correctly, step2 should be logged as reentrancy is enabled.
Correct me if I'm wrong. Thanks.

import { AbstractActor, DaprServer, ActorProxyBuilder, CommunicationProtocolEnum, DaprClient, ActorId } from '@dapr/dapr';

const daprHost = process.env.DAPR_HOST || '0.0.0.0';
const daprPort = process.env.DAPR_HTTP_PORT || '3500';
const serverHost = '0.0.0.0';
const serverPort = '3000';

const server = new DaprServer(serverHost, serverPort, daprHost, daprPort, CommunicationProtocolEnum.HTTP, {
  actor: {
    reentrancy: {
      enabled: true,
    },
  },
});

const client = new DaprClient(daprHost, daprPort, CommunicationProtocolEnum.HTTP);

class ExampleActor extends AbstractActor {
  public async step1() {
    console.log('step1');
    const actorId = this.getActorId();
    const builder = new ActorProxyBuilder(ExampleActor, this.getDaprClient());
    const actor = builder.build(actorId);
    await actor.step2();
  }

  public async step2() {
    console.log('step2');
  }

  public async onActivate(): Promise<void> {
    console.log('on activated');
  }

  public async onDeactivate(): Promise<void> {
    console.log('deactivated');
  }
}

(async () => {
  await server.actor.registerActor(ExampleActor);
  await server.actor.init();
  await server.start();
  await new Promise((resolve) => setTimeout(resolve, 5 * 1000));
  const resRegisteredActors = await server.actor.getRegisteredActors();
  console.log(
    `Registered Actor Types: ${JSON.stringify(resRegisteredActors)}`,
  );
  const builder = new ActorProxyBuilder(ExampleActor, client);
  const actor = builder.build(ActorId.createRandomId());
  await actor.step1();
})();

components

---
apiVersion: dapr.io/v1alpha1
kind: Component
metadata:
  name: statestore
  namespace: default
spec:
  type: state.redis
  version: v1
  metadata:
  - name: redisHost
    value: "localhost:6379"
  - name: redisPassword
    value: ""
  - name: actorStateStore
    value: "true"
---
apiVersion: dapr.io/v1alpha1
kind: Component
metadata:
  name: lock
  namespace: default
spec:
  type: lock.redis
  version: v1
  metadata:
  - name: redisHost
    value: "localhost:6379"
  - name: redisPassword
    value: ""
---
@raymondsze
Copy link
Author

I traced the issue a little bit, the js-sdk didn't preserve the headers if actor is called inside the Actor.
And, it is unable to obtain the request header through the abstract actor interface.

My workaround is like this... generate the dapr-reentrancy-id.... and pass it (as header and body) to the actor.
The actor call itself using axios instead of the sdk to cutsomize the dapr-reentrancy-id. header.....

import axios from 'axios';
import * as uuid from 'uuid';

const daprHost = process.env.DAPR_HOST || '0.0.0.0';
const daprPort = process.env.DAPR_HTTP_PORT || '3500';
const daprUrl = `http://${daprHost}:${daprPort}/v1.0`;
const daprClient = axios.create({ baseURL: daprUrl });

import {
  AbstractActor,
  DaprServer,
  CommunicationProtocolEnum,
  ActorId,
} from '@dapr/dapr';

const serverHost = '0.0.0.0';
const serverPort = '4000';

const server = new DaprServer(
  serverHost,
  serverPort,
  daprHost,
  daprPort,
  CommunicationProtocolEnum.HTTP,
  {
    actor: {
      reentrancy: {
        enabled: true,
        maxStackDepth: 5,
      },
    },
  },
);

class ExampleActor extends AbstractActor {
  public async step1(payload: { reentrancyId: string }) {
    console.log(payload);
    const actorId = this.getActorId();
    await daprClient.post(
      `/actors/ExampleActor/${actorId.getId()}/method/step2`,
      {},
      { headers: { 'dapr-reentrancy-id': payload.reentrancyId } },
    );
  }

  public async step2() {
    console.log('step2');
  }

  public async onActivate(): Promise<void> {
    console.log('on activated');
  }

  public async onDeactivate(): Promise<void> {
    console.log('deactivated');
  }
}

(async () => {
  await server.actor.registerActor(ExampleActor);
  await server.actor.init();
  await server.start();
  await new Promise((resolve) => setTimeout(resolve, 5 * 1000));
  const resRegisteredActors = await server.actor.getRegisteredActors();
  console.log(`Registered Actor Types: ${JSON.stringify(resRegisteredActors)}`);
  const actorId = ActorId.createRandomId();
  const reentrancyId = uuid.v4();
  await daprClient.post(`/actors/ExampleActor/${actorId.getId()}/method/step1`, { reentrancyId }, {
    headers: { 'dapr-reentrancy-id': reentrancyId },
  });
})();

@shubham1172
Copy link
Member

Fixing this requires a way to pass reentrancy ID from the HTTPServer implementation of actor to the actor proxy. This needs some re-structuring, and will be targeted in v3.0.0 when we decouple server and client.

@dapr-bot
Copy link
Collaborator

This issue has been automatically marked as stale because it has not had activity in the last 60 days. It will be closed in the next 7 days unless it is tagged (pinned, good first issue, help wanted or triaged/resolved) or other activity occurs. Thank you for your contributions.

@dapr-bot dapr-bot added the stale label Dec 20, 2022
@shubham1172
Copy link
Member

/active

@jeremylcarter
Copy link

jeremylcarter commented Dec 19, 2023

@raymondsze

What I have done in the meantime is monkey patch the ActorClientHTTP invoke function. I will remove the patch when the feature is completed. It's not a great solution, but it gets us out of trouble.

 setupReentrancy() {
    // Here we are patching the ActorClientHTTP to support reentrancy using the `Dapr-Reentrancy-Id` header
    // All subsequent calls in a request chain must use the same correlation/reentrancy ID for the reentrancy to work
    ActorClientHTTP.prototype.invoke = async function (
      actorType: string,
      actorId: ActorId,
      methodName: string,
      body: any,
      reentrancyId?: string,
    ) {
      const urlSafeId = encodeURIComponent(actorId.getId());
      const result = await this.client.execute(`/actors/${actorType}/${urlSafeId}/method/${methodName}`, {
        method: 'POST', // we always use POST calls for Invoking (ref: https://github.com/dapr/js-sdk/pull/137#discussion_r772636068)
        body,
        headers: {
          'Dapr-Reentrancy-Id': reentrancyId,
        },
      });
      return result as object;
    };
  }

@jeremylcarter
Copy link

Shall I make a PR to add reentrancyId as an optional field for invoke for future use?

@shubham1172
Copy link
Member

Hi @jeremylcarter, please feel free to send in a PR. You can add a TODO comment with this issue reference to remove the monkey patch eventually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triaged/resolved
Projects
None yet
Development

No branches or pull requests

6 participants