-
Notifications
You must be signed in to change notification settings - Fork 149
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
[#2062][FOLLOWUP]Improvement(dashboard): Display detailed info for application in dashboard ui #2063
Conversation
This looks a huge change, could you help check this? @maobaolong If this has been applied in your internal cluster, please let me know, this will strength my approval confidence. |
@zuston yeah, this has been applied to our internal codebase and deployed in our RSS cluster already. |
Ping @zuston @jerqi @maobaolong |
Could we use rocksDB to store the history data? If we load the history data, maybe we can't start coordinator quickly. |
I am not good at rocksDB. Is it possible to open a new PR to optimize this using rocksDB? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lwllvyb Thanks for this improvement, it makes us easy to insight the detail of application anytime. Left minor comment inline.
@@ -29,4 +29,4 @@ rss.server.flush.localfile.threadPool.size 10 | |||
rss.server.flush.hadoop.threadPool.size 60 | |||
rss.server.disk.capacity 1g | |||
rss.server.single.buffer.flush.enabled true | |||
rss.server.single.buffer.flush.threshold 128m | |||
rss.server.single.buffer.flush.threshold 128m |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert this change
<el-table-column label="Version" min-width="180"> | ||
<template v-slot="{ row }"> | ||
<div class="mb-4"> | ||
{{ row.version }}_{{ row.gitCommitId }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what I want! 👍
You should have a common abstraction both for file and Rocksdb first. What's more, you should reply my second question, f we load the history data, maybe we can't start coordinator quickly. How to handle this case. |
Next, I will split this PR into more independent PRs and deal with the abstraction suggestion. |
I can't see the logic to read part of history data. And although we only read part of data, the start time will still be long. |
What changes were proposed in this pull request?
Why are the changes needed?
(Please clarify why the changes are needed. For instance,
Fix: #2062
Does this PR introduce any user-facing change?
(Please list the user-facing changes introduced by your change, including
Yes.
How was this patch tested?
(Please test your changes, and provide instructions on how to test it: