Basic abstraction for data-source extension

[louyuting]

Issue Description

Type: feature request

Describe what feature you want

Define datasource extension framework
implement FileRefreshableDataSource

Additional context

Add any other context or screenshots about the feature request here.

[gorexlv]

About the design of datasource extension and property listener, I suggest that split the config converter into two parts:

1. The datasource is responsible for parsing config bytes to support multiple data formats such as yaml,toml,ini,hcl etc. in the future, which means datasource holds config parsing.
2. The rulemanager is responsible for unmarshaling parsed config into config struct to avoid passing interface{} between datasouce/rulemanager, which means rulemanager holds config unmashaling.

Follow this idea, I have an implementation as a reference: gorexlv@562532a

After splitting converter, I implement etcdv3 datasource and file datasource, it is very simple and intuitive.

One small suggestion, looking forwarding more ideas.

@sczyh30 @louyuting

[gorexlv]

A little question: whether each *RuleManager needs a separate datasouce instance?
In another words, How can I put the FlowRule and SystemRule configurations together?
I looked sentinel-java and it seemed impossible, Am I wrong?

@sczyh30

[louyuting]

Here is the Draft Design about datasource extension framework: https://github.com/alibaba/sentinel-golang/wiki/Dynamic-DataSource-Extension-Framework-Design(DRAFT)

Discussions are welcomed.

[louyuting]

About the design of datasource extension and property listener, I suggest that split the config converter into two parts:

1. The datasource is responsible for parsing config bytes to support multiple data formats such as yaml,toml,ini,hcl etc. in the future, which means datasource holds config parsing.
2. The rulemanager is responsible for unmarshaling parsed config into config struct to avoid passing interface{} between datasouce/rulemanager, which means rulemanager holds config unmashaling.

Follow this idea, I have an implementation as a reference: gorexlv@562532a

After splitting converter, I implement etcdv3 datasource and file datasource, it is very simple and intuitive.

One small suggestion, looking forwarding more ideas.

@sczyh30 @louyuting

From my perspective, in current design, rules channel of xxx_manager.go might be redundant. There are some reasons:

1. Updating xxx_manager's rule list is not frequent operation, there is no concurrency concern. Using a channel to update rules might be redundant
2. It's better to keep the scope of the xxx_manager pure, xxx_manager provide the unified ability to update rules(through calling LoadRules function) and don't care about how to update the upper layer.
3. Coupling different update way may not be very elegant in the xxx_manager.
4. If user don't use dynamic datasource, the logic related to dynamic datasource shouldn't be integrated in src/core/xxx/manager.

Discussions are welcomed.

[louyuting]

A little question: whether each *RuleManager needs a separate datasouce instance?
In another words, How can I put the FlowRule and SystemRule configurations together?
I looked sentinel-java and it seemed impossible, Am I wrong?

@sczyh30

From my perspective, through one type dynamic data source to update Manager's rules need one datasource instance. And different types of data source property updates should not be coupled together.

There are some special scenario:

1. there may be multiple managers(for example: flow, config, system) that need to listen on the same property. In this scenario, instance only one dynamic data source make sense.

Discussions are welcomed.

[gorexlv]

1. Each rule manager corresponds to one property/rule config (array), that is fine. I rather doubt whether it must be one manager binding one datasource, that is not friendly to biz-caller who has to initialize multiple datasouces.
2. I agree that updating rules by channel looks redundant, I think we can remove it. But should be careful with update-blocking.
3. RegisterDatasouceListener is equally important to manager as LoadRules. If just keep LoadRules, who and how do we update the Property for dynamic datasources will be a little annoyance. Here I want to mention that calling an exported function from same package is not a good practice. Of course, this can be discussed separately. Anyway, I think it would be better to follow the java design and then handle the generics-lack with care.
4. Whether Coupling different update way depends on how to design the relationship between Manager and Datasouce, as I listed in 1

:)

[louyuting]

@gorexlv My idea about your replies:

1. It should be that one property corresponds to multi rule manager(array). right?
2. Yes, we should careful about update-locking, using the RWMutex is ok I think.
3. Register Datasource Listener is equally important to manager as LoadRules, I agree with it. The main point we're discussing is whether a listener-like mechanism should be on the xxx_manager side or the DataSource side.

It's important to emphasize that your idea about avoiding to pass interface{} parameter is very helpful.

[louyuting]

I think we should have a detailed design document to illustrate the idea.

Please let me know If you have any suggestion about my design, [dynamic data source extension design(DRAFT)](https://github.com/alibaba/sentinel-golang/wiki/Dynamic-DataSource-Extension-Framework-Design(DRAFT, WIP)),

[gorexlv]

should be that one property corresponds to multi rule manager(array). right?

Yes. Mechanically, one data-source, multi rule manager should be supported, or not be forbidden at least.
For example, we strictly limit the connection number to etcd server in our company, because of too many long connections will affect etcd performance, especially in large cluster. One compromise way is that multiple datasources share one connection, but it is a little unfriendly to biz-caller.

Strategically, one data-source to one rule manager would be ok in current, in order to compatible with Java design. But some redundancy for the future is necessary.

Please let me know If you have any suggestion about my design, [dynamic data source extension design(DRAFT)]

Thank you for your work. I'm a little confused about the design of PropertyHandlePair, but it would be nice if there are some codes to help me understand.

:)

[louyuting]

Thanks for reaching out.

For example, we strictly limit the connection number to etcd server in our company, because of too many long connections will affect etcd performance, especially in large cluster. One compromise way is that multiple datasources share one connection, but it is a little unfriendly to biz-caller.

From my point of view, a machine sharing a connection doesn't solve this problem, but it coupling the code. For this scenario, we may need another solution to optimize it.

The latest wiki: https://github.com/alibaba/sentinel-golang/wiki/Dynamic-DataSource-Extension-Framework-Design(DRAFT)
The Draft PR: https://github.com/alibaba/sentinel-golang/pull/73/files

[sczyh30]

A little question: whether each *RuleManager needs a separate datasouce instance?
In another words, How can I put the FlowRule and SystemRule configurations together?
I looked sentinel-java and it seemed impossible, Am I wrong?

Actually Sentinel does not restrict how users parse the data from the config center, so it's okay to put all kinds of rules together in the same dataId (though not recommended).

[gorexlv]

@louyuting thanks for your work. here is a simple question: https://github.com/alibaba/sentinel-golang/pull/73/files#r386189522. I provided a simple resolution in #74 and associated commit, hope it makes some sense.

From my point of view, a machine sharing a connection doesn't solve this problem, but it coupling the code. For this scenario, we may need another solution to optimize it.

@louyuting In a large cluster, the benefits of removing a long connection are objective, especially to the push performance of etcd server. but maybe not significant for other datasouce, such as file.
Also, I don't think it will coupling code, it's depending on the abstraction relationship between datasource and manager which I list above.

Some humble opinions, but I don't insist on it. Looking forwarding a good trade-off :)

Actually Sentinel does not restrict how users parse the data from the config center, so it's okay to put all kinds of rules together in the same dataId (though not recommended).

@sczyh30 thanks, I'll have a try.

[louyuting]

@louyuting thanks for your work. here is a simple question: https://github.com/alibaba/sentinel-golang/pull/73/files#r386189522. I provided a simple resolution in #74 and associated commit, hope it makes some sense.

From my point of view, a machine sharing a connection doesn't solve this problem, but it coupling the code. For this scenario, we may need another solution to optimize it.

@louyuting In a large cluster, the benefits of removing a long connection are objective, especially to the push performance of etcd server. but maybe not significant for other datasouce, such as file.
Also, I don't think it will coupling code, it's depending on the abstraction relationship between datasource and manager which I list above.

Some humble opinions, but I don't insist on it. Looking forwarding a good trade-off :)

Actually Sentinel does not restrict how users parse the data from the config center, so it's okay to put all kinds of rules together in the same dataId (though not recommended).

@sczyh30 thanks, I'll have a try.

Thanks for reaching out~
Maybe my expression is not clear. What I want to say is that sharing a connection per machine in a large cluster could reduce the long connection, I totally agree and that's very helpful and good idea. And in some connection-sensitive scenarios, we may want to further reduce the number of connections through some design, of course, we need to discuss more.

[gorexlv]

here is my implementation: gorexlv@562532a

I haven't PR it yet, because I'm not satisfied with some things. But I hope it helps :)

[louyuting]

here is my implementation: gorexlv@562532a

I haven't PR it yet, because I'm not satisfied with some things. But I hope it helps :)

That's very great. I'll take a closer look.

@louyuting thanks for your work. here is a simple question: https://github.com/alibaba/sentinel-golang/pull/73/files#r386189522. I provided a simple resolution in #74 and associated commit, hope it makes some sense.

Your suggestion is very helpful. And I had replied some comments. I am looking forward to your reply.

[gorexlv]

@louyuting Sorry maybe my replies are not clear. I'll try to explain.

The main point we're discussing is whether a listener-like mechanism should be on the xxx_manager side or the DataSource side

I think DataSource-side is more closer to java design.

Please let me know If you have any suggestion about my design

I agree with the draft. But I pay more attention to some details, and how to follow java design which is important for code iteration and maintains.

I update my implementation: gorexlv@b4c7ec5. I think it partially solves the generics-lack problem in golang, and is close to java design, hopes it helps!

But actually I am still confused with how to put all kinds of rules together in the same dataId ^_^ @sczyh30
I'll PR it, review and comment is welcome!

:)

[gorexlv]

How about explicitly define a RuleManager struct? Maybe a little bit better :)

[louyuting]

@louyuting Sorry maybe my replies are not clear. I'll try to explain.

The main point we're discussing is whether a listener-like mechanism should be on the xxx_manager side or the DataSource side

I think DataSource-side is more closer to java design.

Please let me know If you have any suggestion about my design

I agree with the draft. Actually, I pay more attention to some details and how to follow java design which I think it's important for code iteration and maintains.

I update my implementation: gorexlv@b4c7ec5. I think it partially solves the generic-lack problem in golang, and closer to java design, hope it helps!

But actually I am still confused with how to put all kinds of rules together in the same dataId ^_^ @sczyh30
I'll PR it, review and comment is welcome!

:)

In fact, following java design is not recommended. We should follow golang style design. Now, there's some redundancy in the design of Java. I think we should design more clearly in golang and then refactor the Java version in the future.

Thanks

[gorexlv]

_ I thought Java design was pretty good, except the datasource which makes me a little confused ~~

[gorexlv]

Anyway, looking forward to releasing ASAP :)

[gorexlv]

any progress？ We look forward to introducing it into our libraries :)

[louyuting]

Discussion is going on.
Conclusion about the dynamic dataSource extension framework is expected this week.

Thanks.

[sczyh30]

Resolved in #73.

If you have any other suggestions, feel free to discuss with the community :)