feat: introduce handshake to client and gRPC server #42

whynowy · 2023-04-04T06:27:14Z

Description

This PR introduces a handshake to the client (numa container) and the gRPC server (UDF/UDSink), it is used to provide the server information such as protocol (UDS, TCP), language, version (SDK), etc. The client (Numaflow) is supposed to
get the information before starting gRPC connections. These information can be used to:

Determine what is the right protocol to connect to the gRPC server.
What is the language and the version of the SDK (for backward compatibility purpose)

File-based information exchange is done before starting the gRPC server.

Similarly in Python and Java SDKs, we need to implement same mechanism, and populate the right information.

In terms of checking lib version at runtime:

Python

>>> from importlib.metadata import version
>>> version('lxml')
'4.3.1'

Java

# Make sure the correct version information is stored in jar-file MANIFEST.MF
getClass().getPackage().getImplementationVersion()

NodeJS is supposed to have a way to get the version info.
https://stackoverflow.com/questions/29741373/check-package-version-at-runtime-in-nodejs

Signed-off-by: Derek Wang <[email protected]>

ashwinidulams · 2023-04-04T08:00:27Z

do we have explicit ordering for udf and numa containers today?

vigith · 2023-04-04T15:14:15Z

pkg/info/types.go

+	Go     Language = "go"
+	Python Language = "python"
+	Java   Language = "java"


why do we need to track each language on numaflow-go?

Our sdk versions are not consistent. We should have a mapping in numaflow code to track all the versions for compatibility check.

vigith · 2023-04-04T15:20:17Z

The info-server is used only in the bootup; will it have any purpose after that? If this is only for a one-time information exchange, should we just pass it over a file or something as an initial handshake mechanism?

whynowy · 2023-04-04T15:21:17Z

do we have explicit ordering for udf and numa containers today?

There's no starting order for normal containers, so our numa container does a waituntilready during startup.

whynowy · 2023-04-04T16:32:33Z

The info-server is used only in the bootup; will it have any purpose after that? If this is only for a one-time information exchange, should we just pass it over a file or something as an initial handshake mechanism?

It works with file. Thinking shall we get it integrated with the prometheus support together numaproj/numaflow#324.

vigith · 2023-04-04T16:47:47Z

The info-server is used only in the bootup; will it have any purpose after that? If this is only for a one-time information exchange, should we just pass it over a file or something as an initial handshake mechanism?

It works with file. Thinking shall we get it integrated with the prometheus support together numaproj/numaflow#324.

I do not think it is wise to integrate numaproj/numaflow#324 since it is very opinionated. Let's keep this as lightweight as possible so that the resource usage will be minimal and only on the bootup.

Signed-off-by: Derek Wang <[email protected]>

vigith · 2023-04-11T01:22:54Z

pkg/info/server_info.go

+)
+
+func GetSDKVersion() string {
+	version := "unknown"


is an empty string a better value than "unknown"?

vigith · 2023-04-11T01:25:36Z

pkg/info/server_info.go

+	// Also check if the file is ready to read
+	// It takes some time for the server to write the server info file
+	// TODO: use a better way to wait for the file to be ready
+	b, err := os.ReadFile(options.svrInfoFilePath)


will a file.stat > 0 length be a good check?

vigith · 2023-04-11T01:31:07Z

pkg/info/server_info.go

+			return ctx.Err()
+		default:
+			time.Sleep(100 * time.Millisecond)
+			b, err = os.ReadFile(options.svrInfoFilePath)


how do we handle partial/in-progress writes, eg. numaflow starting reading early before the entire file has been flushed?

Changed to write an END mark.

vigith · 2023-04-11T01:33:28Z

pkg/info/server_info.go

+	// TODO: use a better way to wait for the file to be ready
+	retry := 0
+	b, err := os.ReadFile(options.svrInfoFilePath)
+	for len(b) == 0 && err == nil && retry < 10 {


will we break the loop too early because we read partial data?

Signed-off-by: Derek Wang <[email protected]>

Java implementation of numaproj/numaflow-go#42 Signed-off-by: Keran Yang <[email protected]>

feat: introduce info server

0233801

Signed-off-by: Derek Wang <[email protected]>

whynowy requested a review from a team April 4, 2023 06:52

vigith reviewed Apr 4, 2023

View reviewed changes

Merge remote-tracking branch 'origin/main' into info

95a439e

whynowy marked this pull request as draft April 5, 2023 15:17

whynowy added 5 commits April 10, 2023 15:43

switched to use file

b25227c

Merge remote-tracking branch 'origin/main' into info

5ec8847

.

d0fd39a

Signed-off-by: Derek Wang <[email protected]>

Merge remote-tracking branch 'origin/main' into info

88b91da

.

4d45adf

Signed-off-by: Derek Wang <[email protected]>

whynowy marked this pull request as ready for review April 10, 2023 23:52

whynowy requested a review from vigith April 10, 2023 23:52

whynowy changed the title ~~feat: introduce info server~~ feat: introduce handshake to client and gRPC server Apr 10, 2023

vigith reviewed Apr 11, 2023

View reviewed changes

.

ea17891

Signed-off-by: Derek Wang <[email protected]>

vigith approved these changes Apr 11, 2023

View reviewed changes

whynowy merged commit 7775e68 into main Apr 11, 2023

whynowy deleted the info branch April 11, 2023 16:47

KeranYang mentioned this pull request Apr 13, 2023

feat: introduce handshake to client and gRPC server numaproj/numaflow-java#35

Closed

KeranYang added a commit to KeranYang/numaflow-java that referenced this pull request Apr 13, 2023

feat: introduce handshake to client and gRPC server

2a30ed4

Java implementation of numaproj/numaflow-go#42 Signed-off-by: Keran Yang <[email protected]>

KeranYang mentioned this pull request Apr 13, 2023

feat: introduce handshake to client and gRPC server numaproj/numaflow-java#36

Merged

KeranYang added a commit to numaproj/numaflow-java that referenced this pull request Apr 14, 2023

feat: introduce handshake to client and gRPC server (#36)

33b9bc9

Java implementation of numaproj/numaflow-go#42 Signed-off-by: Keran Yang <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: introduce handshake to client and gRPC server #42

feat: introduce handshake to client and gRPC server #42

whynowy commented Apr 4, 2023 •

edited by vigith

Loading

ashwinidulams commented Apr 4, 2023

vigith Apr 4, 2023

whynowy Apr 4, 2023

vigith commented Apr 4, 2023

whynowy commented Apr 4, 2023

whynowy commented Apr 4, 2023

vigith commented Apr 4, 2023

vigith Apr 11, 2023

whynowy Apr 11, 2023

vigith Apr 11, 2023

vigith Apr 11, 2023

whynowy Apr 11, 2023

vigith Apr 11, 2023

feat: introduce handshake to client and gRPC server #42

feat: introduce handshake to client and gRPC server #42

Conversation

whynowy commented Apr 4, 2023 • edited by vigith Loading

Description

ashwinidulams commented Apr 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vigith commented Apr 4, 2023

whynowy commented Apr 4, 2023

whynowy commented Apr 4, 2023

vigith commented Apr 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whynowy commented Apr 4, 2023 •

edited by vigith

Loading