You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/07/15 19:40:08 UTC

[GitHub] [druid] vogievetsky opened a new pull request #11450: Web console: better handle BigInt math

vogievetsky opened a new pull request #11450:
URL: https://github.com/apache/druid/pull/11450


   There is an issue that the web console will parse BigInts from the server response but BigInts can be tricky to do math with:
   
   ![image](https://user-images.githubusercontent.com/177816/125846858-3bb692e9-a675-4ac4-814b-da1081f5791f.png)
   
   They do not like to interact with other numbers.
   
   This issue could affect several places (we are saved by the fact that most numbers coming back for the service views are not bigint sized - 16+ digits) but one particular place where this cropped up is setting `maxSize` for a historical to over `1 PB` or `1000000000000000` (one followed by 15 zeros).
   
   In this PR I added a TypeScript type `NumberLike = number | BigInt` and then used it all over the place where we *might* get a BigInt from the Druid API. TS naturally flags all the arithmetic issues that need an explicit `Number(...)` conversion.
   
   So no we can have petabyte scale servers:
   
   ![image](https://user-images.githubusercontent.com/177816/125847486-db2ceca3-b4fd-4fd3-ac70-7d586f7ae081.png)
   
   This PR has:
   - [x] been self-reviewed.
   - [x] been tested in a test Druid cluster.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] vogievetsky merged pull request #11450: Web console: better handle BigInt math

Posted by GitBox <gi...@apache.org>.
vogievetsky merged pull request #11450:
URL: https://github.com/apache/druid/pull/11450


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] suneet-s commented on a change in pull request #11450: Web console: better handle BigInt math

Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #11450:
URL: https://github.com/apache/druid/pull/11450#discussion_r672802079



##########
File path: web-console/src/views/datasource-view/datasource-view.tsx
##########
@@ -1141,10 +1142,10 @@ ORDER BY 1`;
                   day_aligned_segments,
                   month_aligned_segments,
                   year_aligned_segments,
-                } = original;
+                } = original as Datasource;
                 const segmentGranularities: string[] = [];
                 if (!num_segments || isNaN(year_aligned_segments)) return '-';
-                if (num_segments - minute_aligned_segments) {
+                if (Number(num_segments) - minute_aligned_segments) {

Review comment:
       should this be cast to `NumberLike` because of line 156?

##########
File path: web-console/src/views/datasource-view/datasource-view.tsx
##########
@@ -224,7 +225,7 @@ interface RetentionDialogOpenOn {
 
 interface CompactionDialogOpenOn {
   readonly datasource: string;
-  readonly compactionConfig: CompactionConfig;
+  readonly compactionConfig?: CompactionConfig;

Review comment:
       Why this change and the ` | undefined` below?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] vogievetsky commented on a change in pull request #11450: Web console: better handle BigInt math

Posted by GitBox <gi...@apache.org>.
vogievetsky commented on a change in pull request #11450:
URL: https://github.com/apache/druid/pull/11450#discussion_r673265385



##########
File path: web-console/src/views/datasource-view/datasource-view.tsx
##########
@@ -224,7 +225,7 @@ interface RetentionDialogOpenOn {
 
 interface CompactionDialogOpenOn {
   readonly datasource: string;
-  readonly compactionConfig: CompactionConfig;
+  readonly compactionConfig?: CompactionConfig;

Review comment:
       Because a CompactionDialog can be open on a datasource without a compaction config (like when one is being created for the first time). This was already being set to `undefined` in some places, this change just makes the issue more explicit.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] suneet-s commented on a change in pull request #11450: Web console: better handle BigInt math

Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #11450:
URL: https://github.com/apache/druid/pull/11450#discussion_r672802229



##########
File path: web-console/src/views/datasource-view/datasource-view.tsx
##########
@@ -224,7 +225,7 @@ interface RetentionDialogOpenOn {
 
 interface CompactionDialogOpenOn {
   readonly datasource: string;
-  readonly compactionConfig: CompactionConfig;
+  readonly compactionConfig?: CompactionConfig;

Review comment:
       Why this change and the ` | undefined` below?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] suneet-s commented on a change in pull request #11450: Web console: better handle BigInt math

Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #11450:
URL: https://github.com/apache/druid/pull/11450#discussion_r672802079



##########
File path: web-console/src/views/datasource-view/datasource-view.tsx
##########
@@ -1141,10 +1142,10 @@ ORDER BY 1`;
                   day_aligned_segments,
                   month_aligned_segments,
                   year_aligned_segments,
-                } = original;
+                } = original as Datasource;
                 const segmentGranularities: string[] = [];
                 if (!num_segments || isNaN(year_aligned_segments)) return '-';
-                if (num_segments - minute_aligned_segments) {
+                if (Number(num_segments) - minute_aligned_segments) {

Review comment:
       should this be cast to `NumberLike` because of line 156?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] suneet-s commented on a change in pull request #11450: Web console: better handle BigInt math

Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #11450:
URL: https://github.com/apache/druid/pull/11450#discussion_r672802079



##########
File path: web-console/src/views/datasource-view/datasource-view.tsx
##########
@@ -1141,10 +1142,10 @@ ORDER BY 1`;
                   day_aligned_segments,
                   month_aligned_segments,
                   year_aligned_segments,
-                } = original;
+                } = original as Datasource;
                 const segmentGranularities: string[] = [];
                 if (!num_segments || isNaN(year_aligned_segments)) return '-';
-                if (num_segments - minute_aligned_segments) {
+                if (Number(num_segments) - minute_aligned_segments) {

Review comment:
       should this be cast to `NumberLike` because of line 156?

##########
File path: web-console/src/views/datasource-view/datasource-view.tsx
##########
@@ -224,7 +225,7 @@ interface RetentionDialogOpenOn {
 
 interface CompactionDialogOpenOn {
   readonly datasource: string;
-  readonly compactionConfig: CompactionConfig;
+  readonly compactionConfig?: CompactionConfig;

Review comment:
       Why this change and the ` | undefined` below?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] vogievetsky commented on pull request #11450: Web console: better handle BigInt math

Posted by GitBox <gi...@apache.org>.
vogievetsky commented on pull request #11450:
URL: https://github.com/apache/druid/pull/11450#issuecomment-883788841


   Woop woop! Thank you for the review.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org