Skip to content

Change AcknowledegmentMessage interface#4898

Merged
style95 merged 3 commits intoapache:masterfrom
KeonHee:feature/ack-msg-interface
Jun 3, 2020
Merged

Change AcknowledegmentMessage interface#4898
style95 merged 3 commits intoapache:masterfrom
KeonHee:feature/ack-msg-interface

Conversation

@KeonHee
Copy link
Copy Markdown
Member

@KeonHee KeonHee commented May 8, 2020

Description

  1. instanceType json field added
    • It is necessary when another component is added to distinguish json values of invoker and controller.
  2. Add test case for serde InstanceId
  3. Change AcknowledegmentMessage interface
    • The AcknowledegmentMessage may not always respond to the invoker

Related issue and scope

no related issue

My changes affect the following components

  • API
  • Controller
  • Message Bus (e.g., Kafka)
  • Loadbalancer
  • Invoker
  • Intrinsic actions (e.g., sequences, conductors)
  • Data stores (e.g., CouchDB)
  • Tests
  • Deployment
  • CLI
  • General tooling
  • Documentation

Types of changes

  • Bug fix (generally a non-breaking change which closes an issue).
  • Enhancement or new feature (adds new functionality).
  • Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

  • I signed an Apache CLA.
  • I reviewed the style guides and followed the recommendations (Travis CI will check :).
  • I added tests to cover my changes.
  • My changes require further changes to the documentation.
  • I updated the documentation where necessary.

@rabbah
Copy link
Copy Markdown
Member

rabbah commented May 8, 2020

hi @KeonHee can you expand on the need for this change? What components are you adding that are not "invokers" but which are sending ack messages?

@KeonHee
Copy link
Copy Markdown
Member Author

KeonHee commented May 13, 2020

hello @rabbah sorry for late reply

Yes we have a component called "scheduler" between the controller and invoker.
The scheduler is acting as a queue. So, the message that stayed in the queue too long due to lack of resources in the invoker or not is responded back to the controller.

activationsPerNamespace.get(entry.namespaceId).foreach(_.decrement())

releaseInvoker(invoker, entry)
invoker.foreach(releaseInvoker(_, entry))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

If this is to use some methods of CommonLoadBalancer in the downstream, isn't there any other methods to be updated as well?

Copy link
Copy Markdown
Member

@style95 style95 left a comment

Choose a reason for hiding this comment

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

LGTM

@style95 style95 merged commit 8759cad into apache:master Jun 3, 2020
@KeonHee KeonHee deleted the feature/ack-msg-interface branch June 3, 2020 03:20
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

Successfully merging this pull request may close these issues.

3 participants