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

Microservice support #640

Merged
merged 97 commits into from
Jun 14, 2023
Merged

Microservice support #640

merged 97 commits into from
Jun 14, 2023

Conversation

levb
Copy link
Collaborator

@levb levb commented Mar 1, 2023

Description

This PR adds NATS microservice support. See ./examples/micro-hello.c for a very simple NATS microservice implementation.

Contains:

  • microservices framework that support endpoints and monitoring subjects ($SVC.PING, $SVC.INFO, $SVC.STATS, etc.)
  • a simple client to use with microservices.
  • examples:
    • micro-hello is a simple example
    • micro-sequence, micro-func, and micro-arithmetics are a multi-service example that calculates sum(1/func(N)).
    • micro-stats sets a custom stats handler and keeps state in the microservice.
  • misc additions:
    • type microError - microservice-level errors, with a Errorf and Wrapf functions.
    • nats_marshalDuration - formats durations, go-style
    • microArgs a simple space-deparated args parser, for examples, 2/5 prefer to publically expose nats_JSON?

@kozlovic
Copy link
Member

kozlovic commented Mar 7, 2023

Welcome to the NATS C client repo ;-)

I understand that this is still a WIP, but since it will be a new addition likely part of a v3.7.0 release, I would recommend that you locally pull the updated dev branch (I just updated with the latest changes from main) and rebase your work out of that branch and update/resubmit this PR against dev instead of main branch.

I may do a v3.6.1 release (from main) soon since there have been 3 fixes since the last release.

@levb levb changed the base branch from main to dev March 16, 2023 12:15
@levb
Copy link
Collaborator Author

levb commented Mar 27, 2023

@kozlovic I am getting close to having most of the implementation in place. I can really use maybe not yet a thorough review, but a large-scale assessment of the approaches used, and a scan for obvious snafus.

Personally, I feel most controversial about
a) malloc-ed microErrors - they are very handy, but the concept of entrusting/burdening the client with freeing them is definitely questionable. Since microRequest_Respond frees the error, the burden is low, that's the dominant client use-case for error handling, but still...
b) I threw together a simple args parser for the example microservice. It should be replaced with something more robust as the reference point. Suggestions? Protobuf?
c) I still do not fully understand the C and go clients concurrency model. Specifically, AddEndpoint does not lock the service in go, presumably because these are startup-only, but the endpoints start immediately and may start receiving messages before other endpoints are added... still very much figuring things out.

@levb levb changed the title WIP: A skeleton microservice implementation Microservice support Apr 26, 2023
@levb levb marked this pull request as ready for review April 26, 2023 19:18
@levb levb requested review from kozlovic and piotrpio April 26, 2023 19:18
src/nats.h Outdated Show resolved Hide resolved
src/nats.h Outdated Show resolved Hide resolved
src/micro.c Outdated Show resolved Hide resolved
@levb
Copy link
Collaborator Author

levb commented Jun 11, 2023

@kozlovic apologies for not squashing all these commits, I now set "squash-merge" as the default for PRs to avoid it going forward.

Please take a final look at this PR; there is #663 still outstanding but it's just a change in INFO/JSON, passes the tests, I think a cursory look will suffice or just leave it to @piotrpio since he knows the context.

I'd appreciate another Windows compile (on the ^^ branch?) since I still have not been able to set up a functional windows worker build.

@levb levb requested a review from kozlovic June 11, 2023 17:25
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

The test test_MicroAsyncErrorHandler is causing an assert failure on Windows and I have not yet been able to figure out what the issue is. Will continue looking into that when I have some time.

test/test.c Show resolved Hide resolved
test/test.c Show resolved Hide resolved
levb and others added 2 commits June 13, 2023 06:22
* More verbose endpoint INFO

* better clone/free for metadata

* (valgrind) free test JSON array
@levb levb requested a review from kozlovic June 13, 2023 13:26
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

LGTM. Let's merge this and I will get back to the Windows specific issue as a separate PR. Thanks!

@kozlovic
Copy link
Member

@levb Could you please not merge yet? I am going to push a commit that fixes the issues that I found on Windows. Thanks!

The change that I asked to make earlier regarding nats_vsnprint
in some places was wrong, but more importantly, you can't pass
a NULL buffer on Windows in order to calculate the length of
the formatted string. Instead, I added a new macro that uses
a different function for Windows.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
@kozlovic
Copy link
Member

@levb Ok, please review the latest commit I pushed and if it is ok with you, then feel free to merge since I already gave the LGTM for the rest.

@levb levb merged commit 71708ed into dev Jun 14, 2023
2 checks passed
@levb levb deleted the lev-microservice-proto branch June 14, 2023 10:56
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.

None yet

3 participants