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

A grpc kernel that stores everything in a remote grpc kernel service #148

Closed
Tracked by #145
huachaohuang opened this issue Dec 1, 2021 · 23 comments · Fixed by #171 or #193
Closed
Tracked by #145

A grpc kernel that stores everything in a remote grpc kernel service #148

huachaohuang opened this issue Dec 1, 2021 · 23 comments · Fixed by #171 or #193
Milestone

Comments

@huachaohuang
Copy link
Contributor

huachaohuang commented Dec 1, 2021

A grpc kernel integrates a grpc journal, storage, etc. It consists of a client and a server part. We need to provide a binary to start the kernel server.

@huachaohuang huachaohuang mentioned this issue Dec 1, 2021
5 tasks
@huachaohuang huachaohuang added this to the Version 0.2 milestone Dec 1, 2021
@tisonkun
Copy link
Contributor

tisonkun commented Dec 4, 2021

Will the binary be part of engula binary? I'm thinking of delivering a one-for-all binary as cockroachdb does.

@huachaohuang
Copy link
Contributor Author

Will the binary be part of engula binary? I'm thinking of delivering a one-for-all binary as cockroachdb does.

Yes, of course. I hate to operate a bunch of different binaries too.

@huachaohuang
Copy link
Contributor Author

This can be started after #167

@w41ter
Copy link
Contributor

w41ter commented Dec 7, 2021

I am interested in this job, I need some time to learn the details first, and the give my design.

@huachaohuang
Copy link
Contributor Author

@PatrickNicholas Thanks for your interest. It sounds good to me. The job may not be easy, so let me make it more clear:

  • The grpc kernel service should accept another kernel implementation as its backend. For example, we can pass in a memory kernel to get a kernel service that stores everything in memory.
  • We should provide a binary (engula) to start the kernel service.

This can be done in one or two PRs.

Note that this job is included in v0.2, and we aim to release that on Dec 17. So please let me know if you meet any problems. And I may take over if there is no progress until Dec 11.

@w41ter
Copy link
Contributor

w41ter commented Dec 7, 2021

@huachaohuang Thanks.

w41ter added a commit to w41ter/engula that referenced this issue Dec 7, 2021
w41ter added a commit to w41ter/engula that referenced this issue Dec 8, 2021
w41ter added a commit to w41ter/engula that referenced this issue Dec 8, 2021
w41ter added a commit to w41ter/engula that referenced this issue Dec 8, 2021
w41ter added a commit to w41ter/engula that referenced this issue Dec 8, 2021
@w41ter
Copy link
Contributor

w41ter commented Dec 8, 2021

It is sad that this issue was automatically closed, due to the closing keywords set in previous PR. Shall I reopen it?

@tisonkun
Copy link
Contributor

tisonkun commented Dec 8, 2021

@PatrickNicholas aha. I think the next time you can use ref #xxx where ref is not a keyword so that GitHub only show the reference but not close the issue automatically. I'm unaware that you separate the kernel implementation part and the cli part.

@huachaohuang huachaohuang reopened this Dec 8, 2021
w41ter added a commit to w41ter/engula that referenced this issue Dec 9, 2021
@huachaohuang huachaohuang mentioned this issue Dec 11, 2021
10 tasks
@huachaohuang
Copy link
Contributor Author

huachaohuang commented Dec 13, 2021

I think it's more meaningful to allow users to start independent kernel/journal/storage servers and connect them together even for v0.2. The kernel/journal/storage services are already landed, so I think we can deliver such kind of flexibility in v0.2.

For example, we can support the following commands:

engula journal start 127.0.0.1:10001 [--mem | --file path]
engula storage start 127.0.0.1:10002 [--mem | --file path]
engula kernel start 127.0.0.1:10003 --journal 127.0.0.1:10001 --storage 127.0.0.1:10002

Ref #177, @PatrickNicholas what do you think?

/cc @tisonkun I saw your #177 (comment), is this the way you expect?

@w41ter
Copy link
Contributor

w41ter commented Dec 13, 2021

@huachaohuang

In this way, each server is independent, so do we still need to support standalone server, eg: a grpc kernel but with mem journal and mem storage.

@huachaohuang
Copy link
Contributor Author

In this way, each server is independent, so do we still need to support standalone server, eg: a grpc kernel but with mem journal and mem storage.

I think a standalone server is not necessary then. Just let users do whatever they want.

@tisonkun
Copy link
Contributor

@huachaohuang Yes. This CLI design looks reasonable.

@PatrickNicholas for composition topics, we should discuss with a deployment photo to make things clear. "A grpc kernel" data structure is different from "a kernel server". I think if a pod runs a kernel server, generally it doesn't run a memory journal colocated.

@tisonkun
Copy link
Contributor

I think a standalone server is not necessary then. Just let users do whatever they want.

Yes. We prodvides basic blocks, and a standalone server launching script can be a upper-level script, possibly not bundled by the release.

@w41ter
Copy link
Contributor

w41ter commented Dec 13, 2021

Ok, I am glad to add a binary to start journal & storage server too. So #177 have to be delayed until I submit PRs to support both journal & storage server ctl.

@huachaohuang
Copy link
Contributor Author

@PatrickNicholas Great, I will add a file kernel once #185 is merged. And I may update the local kernel a little bit. We may have some little conflicts here. So you can prepare for the binary beforehand or wait until I finish other stuff.

@w41ter
Copy link
Contributor

w41ter commented Dec 13, 2021

So you can prepare for the binary beforehand or wait until I finish other stuff.

I will add storage server first, then add journal server once #185 is merged, so we should be able to do it in parallel.

@huachaohuang
Copy link
Contributor Author

@PatrickNicholas My job is done here. It's your turn now :)

@w41ter
Copy link
Contributor

w41ter commented Dec 13, 2021

engula journal start 127.0.0.1:10001 [--mem | --file path]
engula storage start 127.0.0.1:10002 [--mem | --file path]
engula kernel start 127.0.0.1:10003 --journal 127.0.0.1:10001 --storage 127.0.0.1:10002

@huachaohuang Like Journal and Storage, Kernel may be Mem or File. At the same time, Kernel can also connect to Journal Server and Storage Server. In this case, which type the Manifest should be, Mem or File?

@huachaohuang
Copy link
Contributor Author

How about

engula kernel start <kernel_addr> [--mem | --file path] --journal <journal_addr> --storage <storage_addr>

Not sure if it is too complicated than necessary.

@w41ter
Copy link
Contributor

w41ter commented Dec 13, 2021

Not sure if it is too complicated than necessary.

It's very complicated, that means we have to define types:

type Kernel1 = local::Kernel<mem::Journal, mem::Storage, mem::Manifest>;
type Kernel2 = local::Kernel<grpc::Journal, mem::Storage, mem::Manifest>;
type Kernel3 = local::Kernel<mem::Journal, grpc::Storage, mem::Manifest>;
type Kernel4 = local::Kernel<grpc::Journal, grpc::Storage, mem::Manifest>;
....
type Kerneln = local::Kernel<file::Journal, file::Storage, file::Manifest>;

Maybe we should use dynamic dispatching rather than generic...

@huachaohuang
Copy link
Contributor Author

Oh, I see. Seems these compositions don't play well together. Maybe dynamic dispatching for non-performance-critical types is reasonable. But let's try to work around it for v0.2.

I think we only need the following to implement the said commands?

local::Kernel<grpc::Journal, grpc::Storage, mem::Manifest>
local::Kernel<grpc::Journal, grpc::Storage, file::Manifest>

@w41ter
Copy link
Contributor

w41ter commented Dec 14, 2021

@huachaohuang There is still a problem. A grpc client of Kernel need to find the addresses of Journal server and Storage server which specified in Kernel server arguments, so that access the Journal server and Storage server directly. I am planing to add a RPC PlaceLookup to acquire the addresses. At the same time, a trait named PlaceResolver is added to abstract place lookuping. Then Kernel server will require that generic type shoud satisfied : Kernel + PlaceResolver. Do you have any suggestions?

@huachaohuang
Copy link
Contributor Author

@PatrickNicholas Let's keep it simple for v0.2. We can add RPCs to the kernel service to get stream/bucket locations. The kernel service simply returns the journal/storage service address and then the kernel client can talk to the journal/storage service to read/write stream/bucket directly.

w41ter added a commit to w41ter/engula that referenced this issue Dec 14, 2021
w41ter added a commit to w41ter/engula that referenced this issue Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants