[#1401][part-1] The dashboard supports multiple Coordinator links. #1449

yl09099 · 2024-01-14T15:41:56Z

What changes were proposed in this pull request?

After multiple coordinators are deployed, a dashboard is added to link multiple coordinators.

Why are the changes needed?

Fix: #1401

Does this PR introduce any user-facing change?

No.

How was this patch tested?

No.

codecov-commenter · 2024-01-14T15:55:03Z

Codecov Report

Attention: 43 lines in your changes are missing coverage. Please review.

Comparison is base (b43bbd8) 54.15% compared to head (9f06a14) 54.86%.
Report is 12 commits behind head on master.

Files	Patch %	Lines
...pache/uniffle/dashboard/web/resource/Response.java	0.00%	18 Missing ⚠️
...apache/uniffle/dashboard/web/JettyServerFront.java	0.00%	6 Missing ⚠️
...shboard/web/resource/GainCoordinatorsResource.java	0.00%	6 Missing ⚠️
...e/uniffle/dashboard/web/proxy/WebProxyServlet.java	0.00%	4 Missing ⚠️
...e/uniffle/dashboard/web/resource/BaseResource.java	0.00%	4 Missing ⚠️
...he/uniffle/dashboard/web/utils/DashboardUtils.java	72.72%	3 Missing ⚠️
...he/uniffle/dashboard/web/resource/WebResource.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1449      +/-   ##
============================================
+ Coverage     54.15%   54.86%   +0.70%     
- Complexity     2773     2806      +33     
============================================
  Files           423      415       -8     
  Lines         24178    22141    -2037     
  Branches       2057     2079      +22     
============================================
- Hits          13094    12147     -947     
+ Misses        10275     9236    -1039     
+ Partials        809      758      -51

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

zuston · 2024-01-15T03:41:41Z

Could you help review this? @xianjingfeng

xianjingfeng · 2024-01-15T08:32:18Z

dashboard/src/main/webapp/src/pages/CoordinatorServerPage.vue

+    //The system obtains data from global variables and requests the interface to obtain new data after data changes.
+    const currentServerStore= useCurrentServerStore()
+    currentServerStore.$subscribe((mutable,state)=>{
+      const headrs={"targetAddress":state.currentServer}


Is it better that passing this parameter through cookie？

Is it better that passing this parameter through cookie？

This is global. The role of cookies is not well understood.

It is not graceful to set the header before each request. Some code is redundant.

Or maybe you can use local storage and get targetAddress from the local storage before invoking the http interfaces

@yl09099 @zuston @rickyma PTAL

dashboard/src/main/java/org/apache/uniffle/dashboard/web/proxy/WebProxyServlet.java

dashboard/src/main/java/org/apache/uniffle/dashboard/web/utils/DashboardUtils.java

dashboard/src/main/java/org/apache/uniffle/dashboard/web/views/GainCoordinatorsServlet.java

dashboard/src/main/webapp/src/api/api.js

rickyma · 2024-01-15T13:14:08Z

Can we also fix the following spelling issues in this PR?

My suggesion will be:
ResigerTime -> RegistrationTime
in files:
dashboard/src/main/webapp/src/components/shufflecomponent/ActiveNodeListPage.vue
dashboard/src/main/webapp/src/components/shufflecomponent/DecommissionednodeListPage.vue
dashboard/src/main/webapp/src/components/shufflecomponent/DecommissioningNodeListPage.vue
dashboard/src/main/webapp/src/components/shufflecomponent/LostNodeList.vue
dashboard/src/main/webapp/src/components/shufflecomponent/UnhealthyNodeListPage.vue

@yl09099

yl09099 · 2024-01-21T07:24:10Z

Can we also fix the following spelling issues in this PR?

My suggesion will be: ResigerTime -> RegistrationTime in files: dashboard/src/main/webapp/src/components/shufflecomponent/ActiveNodeListPage.vue dashboard/src/main/webapp/src/components/shufflecomponent/DecommissionednodeListPage.vue dashboard/src/main/webapp/src/components/shufflecomponent/DecommissioningNodeListPage.vue dashboard/src/main/webapp/src/components/shufflecomponent/LostNodeList.vue dashboard/src/main/webapp/src/components/shufflecomponent/UnhealthyNodeListPage.vue

@yl09099

Have changed.

dashboard/src/main/webapp/src/utils/http.js

dashboard/src/main/webapp/src/utils/request.js

xianjingfeng · 2024-01-24T06:24:16Z

dashboard/src/main/webapp/package.json

@@ -32,6 +32,7 @@
    "core-js": "^3.8.3",
    "element-plus": "^2.3.6",
    "moment": "^2.29.4",
+    "pinia": "^2.1.7",


I just learned about pinia. Can the data in the store be shared between tabs of a browser?

I suggest remove pinia, we can just use localStorage. @yl09099 @zuston @rickyma PTAL

rickyma · 2024-04-03T14:26:22Z

No progress for this? I think we need a more powerful dashboard.

zuston · 2024-04-03T15:05:44Z

No progress for this? I think we need a more powerful dashboard.

Do you have thought on this? feel free to discuss more here

rickyma · 2024-04-04T16:48:39Z

No progress for this? I think we need a more powerful dashboard.

Do you have thought on this? feel free to discuss more here

I believe the dashboard should be updated regularly, as it provides the most intuitive experience for users. And it facilitates the management and operation of the RSS cluster for dev and ops.

I think it could be improved by:

@zuston

github-actions · 2024-04-14T11:12:33Z

Test Results

2 358 files - 5 2 358 suites - 5 4h 28m 12s ⏱️ - 2m 14s
913 tests +1 911 ✅ ±0 1 💤 ±0 0 ❌ ±0 1 🔥 +1
10 580 runs - 5 10 565 ✅ - 6 14 💤 ±0 0 ❌ ±0 1 🔥 +1

For more details on these errors, see this check.

Results for commit b4142f6. ± Comparison against base commit 6ea4300.

rickyma · 2024-04-15T13:52:50Z

This is a flaky test. We can ignore this for now. I have already created an issue for this #1628.

It is ready for review? Can anyone review this? We wanna use this on production this month. @yl09099

zuston · 2024-04-16T02:31:59Z

@xianjingfeng Could you help review this?

rickyma · 2024-04-16T12:12:48Z

ping @xianjingfeng

xianjingfeng · 2024-04-16T14:08:24Z

dashboard/src/main/webapp/vue.config.js

@@ -16,4 +16,16 @@
 */

 module.exports ={
+    // 可以通过配置 vue.config.js 文件来设置代理，以将请求代理到后端服务器。


Could you translate this sentence into english?

xianjingfeng · 2024-04-16T14:17:50Z

dashboard/src/main/java/org/apache/uniffle/dashboard/web/resource/GainCoordinatorsResource.java

+
+  @GET
+  @Path("/coordinatorList")
+  public Response getCoordinatorServers() {


Suggested change

public Response getCoordinatorServers() {

public Response<Map<String, String>> getCoordinatorServers() {

xianjingfeng · 2024-04-16T14:19:05Z

dashboard/src/main/java/org/apache/uniffle/dashboard/web/resource/GainCoordinatorsResource.java

+import org.apache.hbase.thirdparty.javax.ws.rs.core.MediaType;
+
+@Produces({MediaType.APPLICATION_JSON})
+public class GainCoordinatorsResource extends BaseResource {


I think CoordinatorsResource is better.

xianjingfeng · 2024-04-16T14:23:47Z

dashboard/src/main/java/org/apache/uniffle/dashboard/web/resource/WebResource.java

+import org.apache.hbase.thirdparty.javax.ws.rs.Produces;
+import org.apache.hbase.thirdparty.javax.ws.rs.core.MediaType;
+
+@Path("web")


I'm not sure if this path is correct. Is the url will be like '/web/coordinator/...'?

xianjingfeng · 2024-04-16T14:26:45Z

dashboard/src/main/webapp/src/router/index.js

@@ -59,6 +59,12 @@ const routes = [
        name: 'applicationpage',
        component: ApplicationPage,
    },
+    {
+        path: '/nullpage',


Is this necessary?

yl09099 force-pushed the uniffle-1141 branch from a2f8e4e to 27ab7c7 Compare January 14, 2024 15:45

yl09099 force-pushed the uniffle-1141 branch 2 times, most recently from 0e92c00 to eecddd0 Compare January 15, 2024 02:21

zuston requested a review from xianjingfeng January 15, 2024 03:41

xianjingfeng reviewed Jan 15, 2024

View reviewed changes

dashboard/src/main/java/org/apache/uniffle/dashboard/web/proxy/WebProxyServlet.java Outdated Show resolved Hide resolved

xianjingfeng reviewed Jan 15, 2024

View reviewed changes

dashboard/src/main/java/org/apache/uniffle/dashboard/web/utils/DashboardUtils.java Outdated Show resolved Hide resolved

xianjingfeng reviewed Jan 15, 2024

View reviewed changes

dashboard/src/main/java/org/apache/uniffle/dashboard/web/views/GainCoordinatorsServlet.java Outdated Show resolved Hide resolved

xianjingfeng reviewed Jan 15, 2024

View reviewed changes

dashboard/src/main/webapp/src/api/api.js Outdated Show resolved Hide resolved

yl09099 force-pushed the uniffle-1141 branch from eecddd0 to 9f06a14 Compare January 21, 2024 07:22

yl09099 requested a review from xianjingfeng January 24, 2024 05:12

xianjingfeng reviewed Jan 24, 2024

View reviewed changes

dashboard/src/main/webapp/src/utils/http.js Outdated Show resolved Hide resolved

xianjingfeng reviewed Jan 24, 2024

View reviewed changes

dashboard/src/main/webapp/src/utils/request.js Outdated Show resolved Hide resolved

xianjingfeng reviewed Jan 24, 2024

View reviewed changes

[apache#1401][part-1] The dashboard supports multiple Coordinator links.

b4142f6

yl09099 force-pushed the uniffle-1141 branch from 9f06a14 to b4142f6 Compare April 14, 2024 10:49

zuston requested a review from xianjingfeng April 16, 2024 02:32

xianjingfeng reviewed Apr 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#1401][part-1] The dashboard supports multiple Coordinator links. #1449

[#1401][part-1] The dashboard supports multiple Coordinator links. #1449

yl09099 commented Jan 14, 2024 •

edited

codecov-commenter commented Jan 14, 2024 •

edited

zuston commented Jan 15, 2024

xianjingfeng Jan 15, 2024

yl09099 Jan 21, 2024

xianjingfeng Jan 24, 2024

xianjingfeng Jan 24, 2024

xianjingfeng Apr 16, 2024

rickyma commented Jan 15, 2024

yl09099 commented Jan 21, 2024

xianjingfeng Jan 24, 2024

xianjingfeng Apr 16, 2024

rickyma commented Apr 3, 2024

zuston commented Apr 3, 2024

rickyma commented Apr 4, 2024 •

edited

github-actions bot commented Apr 14, 2024

rickyma commented Apr 15, 2024

zuston commented Apr 16, 2024 •

edited

rickyma commented Apr 16, 2024

xianjingfeng Apr 16, 2024

xianjingfeng Apr 16, 2024

xianjingfeng Apr 16, 2024

xianjingfeng Apr 16, 2024

xianjingfeng Apr 16, 2024

	public Response getCoordinatorServers() {
	public Response<Map<String, String>> getCoordinatorServers() {

[#1401][part-1] The dashboard supports multiple Coordinator links. #1449

Are you sure you want to change the base?

[#1401][part-1] The dashboard supports multiple Coordinator links. #1449

Conversation

yl09099 commented Jan 14, 2024 • edited

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

codecov-commenter commented Jan 14, 2024 • edited

Codecov Report

zuston commented Jan 15, 2024

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

rickyma commented Jan 15, 2024

yl09099 commented Jan 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rickyma commented Apr 3, 2024

zuston commented Apr 3, 2024

rickyma commented Apr 4, 2024 • edited

github-actions bot commented Apr 14, 2024

Test Results

rickyma commented Apr 15, 2024

zuston commented Apr 16, 2024 • edited

rickyma commented Apr 16, 2024

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

yl09099 commented Jan 14, 2024 •

edited

codecov-commenter commented Jan 14, 2024 •

edited

rickyma commented Apr 4, 2024 •

edited

zuston commented Apr 16, 2024 •

edited