[kudu-CR] [www] Add some guidance to /scans page

2021-09-14 Thread Yifan Zhang (Code Review) 
Hello Kudu Jenkins, Abhishek Chennaka, Andrew Wong,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/17846

to look at the new patch set (#4).

Change subject: [www] Add some guidance to /scans page
..

[www] Add some guidance to /scans page

The scans page looks like this after this change:
http://ww1.sinaimg.cn/large/002Qy9fDgy1guh6cgwpygj62qg0e2dkb02.jpg

Change-Id: I056893c4c5dae63c90d42db8c907cf1646de944e
---
M src/kudu/tserver/tserver_path_handlers.cc
M www/scans.mustache
2 files changed, 9 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/17846/4
--
To view, visit http://gerrit.cloudera.org:8080/17846
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I056893c4c5dae63c90d42db8c907cf1646de944e
Gerrit-Change-Number: 17846
Gerrit-PatchSet: 4
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Abhishek Chennaka 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang

[kudu-CR] [www] Add some guidance to /scans page

2021-09-14 Thread Andrew Wong (Code Review) 
Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17846 )

Change subject: [www] Add some guidance to /scans page
..


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17846/3/www/scans.mustache
File www/scans.mustache:

http://gerrit.cloudera.org:8080/#/c/17846/3/www/scans.mustache@22
PS3, Line 22: h <
nit: add a "the", i.e. "with the --scan_history_count flag."



--
To view, visit http://gerrit.cloudera.org:8080/17846
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I056893c4c5dae63c90d42db8c907cf1646de944e
Gerrit-Change-Number: 17846
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Abhishek Chennaka 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Wed, 15 Sep 2021 03:01:51 +
Gerrit-HasComments: Yes

[kudu-CR] [www] Add some guidance to /scans page

2021-09-14 Thread Yifan Zhang (Code Review) 
Yifan Zhang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17846 )

Change subject: [www] Add some guidance to /scans page
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17846/1/www/scans.mustache
File www/scans.mustache:

http://gerrit.cloudera.org:8080/#/c/17846/1/www/scans.mustache@21
PS1, Line 21: ,
> nit: Probably don't need a comma(,)
Done



--
To view, visit http://gerrit.cloudera.org:8080/17846
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I056893c4c5dae63c90d42db8c907cf1646de944e
Gerrit-Change-Number: 17846
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Abhishek Chennaka 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang 
Gerrit-Comment-Date: Wed, 15 Sep 2021 02:20:01 +
Gerrit-HasComments: Yes

[kudu-CR] wip [rpc] KUDU-75: refresh DNS entries if proxies hit a network error

2021-09-14 Thread Bankim Bhavsar (Code Review) 
Bankim Bhavsar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17839 )

Change subject: wip [rpc] KUDU-75: refresh DNS entries if proxies hit a network 
error
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/17839/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17839/3//COMMIT_MSG@12
PS3, Line 12: - Also unclear whether we should force all Proxies to abide by 
this new
:   implementation, or have a separate class or mode that doesn't
:   re-resolve on failure
I like the idea of a mode using a config that turns on resolving DNS in case of 
error v/s turning it on by default for all proxies.


http://gerrit.cloudera.org:8080/#/c/17839/3/src/kudu/rpc/connection_id.h
File src/kudu/rpc/connection_id.h:

http://gerrit.cloudera.org:8080/#/c/17839/3/src/kudu/rpc/connection_id.h@49
PS3, Line 49: remote
std::move() ?


http://gerrit.cloudera.org:8080/#/c/17839/3/src/kudu/rpc/proxy.cc
File src/kudu/rpc/proxy.cc:

http://gerrit.cloudera.org:8080/#/c/17839/3/src/kudu/rpc/proxy.cc@106
PS3, Line 106: if (PREDICT_TRUE(s.ok() && !addrs.empty())) {
 : addr = addrs[0];
 : DCHECK(addr.is_initialized());
 : addr.set_port(hp_.port());
 :   }
Okay to proceed in case of failure?


http://gerrit.cloudera.org:8080/#/c/17839/3/src/kudu/rpc/proxy.cc@185
PS3, Line 185: successfull
successfully


http://gerrit.cloudera.org:8080/#/c/17839/3/src/kudu/rpc/proxy.cc@201
PS3, Line 201: // TODO(awong): we should be more specific here -- consider 
having the RPC
 : // layer set a flag in the controller that warrants a retry.
 : if (PREDICT_FALSE(controller->status().IsNetworkError())) {
 :   RefreshDnsAndEnqueueRequest(method, req, response, 
controller, callback);
 :   return;
 : }
 : // For any other status, OK or otherwise, just run the 
callback.
 : callback();
What happens if the DNS resolution keeps failing? Will the request be 
indefinitely re-enqueued? Basically when do we give up.



--
To view, visit http://gerrit.cloudera.org:8080/17839
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I777d169bd3a461294e5721f05071b726ced70f7e
Gerrit-Change-Number: 17839
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Bankim Bhavsar 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 14 Sep 2021 22:42:13 +
Gerrit-HasComments: Yes

[kudu-CR] [fs] Refactor fs module

2021-09-14 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17845 )

Change subject: [fs] Refactor fs module
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/17845/1/src/kudu/fs/dir_manager.h
File src/kudu/fs/dir_manager.h:

http://gerrit.cloudera.org:8080/#/c/17845/1/src/kudu/fs/dir_manager.h@254
PS1, Line 254: AllDirFailed
Rename to AllDirsFailed() or AreAllDirsFailed()


http://gerrit.cloudera.org:8080/#/c/17845/1/src/kudu/fs/dir_manager.cc
File src/kudu/fs/dir_manager.cc:

http://gerrit.cloudera.org:8080/#/c/17845/1/src/kudu/fs/dir_manager.cc@227
PS1, Line 227: bool has_healthy_instances = true;
Just curious: why to assign any value here if LoadInstances() do not return OK 
without setting the 'has_healthy_instances' output parameter?


http://gerrit.cloudera.org:8080/#/c/17845/1/src/kudu/fs/fs_report.cc
File src/kudu/fs/fs_report.cc:

http://gerrit.cloudera.org:8080/#/c/17845/1/src/kudu/fs/fs_report.cc@42
PS1, Line 42: MERGE_CHECKS
nit: rename to MERGE_ENTRIES and introduce the 'entries' parameter as well?  
Otherwise, why to expose only the 'other' parameter out of the macro?


http://gerrit.cloudera.org:8080/#/c/17845/1/src/kudu/fs/log_block_manager.cc
File src/kudu/fs/log_block_manager.cc:

http://gerrit.cloudera.org:8080/#/c/17845/1/src/kudu/fs/log_block_manager.cc@2992
PS1, Line 2992:   uint64_t old_metadata_size;
nit: if you moved it closer to the site of its usage, maybe move it one more 
line down right before its usage in the call to env_->GetFileSize()?


http://gerrit.cloudera.org:8080/#/c/17845/1/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:

http://gerrit.cloudera.org:8080/#/c/17845/1/src/kudu/server/server_base.cc@584
PS1, Line 584: LOG(INFO) << s.ToString();
 : LOG(INFO) << "This appears to be a new deployment of Kudu; 
creating new FS layout";
nit: combine these two INFO messages into one?



--
To view, visit http://gerrit.cloudera.org:8080/17845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa45637494b36cc1f4f00affdf995a25eb6231bc
Gerrit-Change-Number: 17845
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 14 Sep 2021 16:34:14 +
Gerrit-HasComments: Yes

[kudu-CR] [www] Add some guidance to /scans page

2021-09-14 Thread Abhishek Chennaka (Code Review) 
Abhishek Chennaka has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17846 )

Change subject: [www] Add some guidance to /scans page
..


Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17846/1/www/scans.mustache
File www/scans.mustache:

http://gerrit.cloudera.org:8080/#/c/17846/1/www/scans.mustache@21
PS1, Line 21: ,
nit: Probably don't need a comma(,)



--
To view, visit http://gerrit.cloudera.org:8080/17846
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I056893c4c5dae63c90d42db8c907cf1646de944e
Gerrit-Change-Number: 17846
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang 
Gerrit-Reviewer: Abhishek Chennaka 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 14 Sep 2021 15:00:28 +
Gerrit-HasComments: Yes

[kudu-CR] [fs] Refactor fs module

2021-09-14 Thread Yingchun Lai (Code Review) 
Yingchun Lai has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/17845


Change subject: [fs] Refactor fs module
..

[fs] Refactor fs module

Only some code style refactoring on fs module, without
any functional changes.

Change-Id: Iaa45637494b36cc1f4f00affdf995a25eb6231bc
---
M src/kudu/fs/block_id.cc
M src/kudu/fs/dir_manager.cc
M src/kudu/fs/dir_manager.h
M src/kudu/fs/file_block_manager.cc
M src/kudu/fs/fs_manager.cc
M src/kudu/fs/fs_manager.h
M src/kudu/fs/fs_report.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/fs/log_block_manager.h
M src/kudu/server/server_base.cc
10 files changed, 91 insertions(+), 99 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/17845/1
--
To view, visit http://gerrit.cloudera.org:8080/17845
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaa45637494b36cc1f4f00affdf995a25eb6231bc
Gerrit-Change-Number: 17845
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai

[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata

2021-09-14 Thread Yingchun Lai (Code Review) 
Yingchun Lai has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/17837 )

Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
..

KUDU-3318 [fs] Add size limit for log block container metadata

In some cases, log block container's data file is very small
after punching hole, while metadata file is relatively large
because it contains many CREATE-DELETE pairs. LBM reclaims
both .data and corresponding .metadata container files when
the .data container file becomes full (i.e. reaches its size
or block count threshold).
So, the .metadata container file might grow without any limit
before the .data container becomes full.
This patch adds a size limit for log block container's metadata,
taking it into account while determining the container's 'full'
condition. That gives the LBM an opportunity to reclaim the disk
space once the container becomes full.

Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Reviewed-on: http://gerrit.cloudera.org:8080/17837
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
Reviewed-by: Andrew Wong 
---
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/pb_util.cc
M src/kudu/util/pb_util.h
4 files changed, 57 insertions(+), 3 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Alexey Serbin: Looks good to me, approved
  Andrew Wong: Looks good to me, approved

--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 9
Gerrit-Owner: Yingchun Lai 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai

[kudu-CR] KUDU-3318 [fs] Add size limit for log block container metadata

2021-09-14 Thread Yingchun Lai (Code Review) 
Yingchun Lai has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17837 )

Change subject: KUDU-3318 [fs] Add size limit for log block container metadata
..


Patch Set 8:

> Patch Set 8: Code-Review+2
>
> (1 comment)

I will plan to implement compact low live block metadata at runtime.


--
To view, visit http://gerrit.cloudera.org:8080/17837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12513abf2e45f7bdf091142c31f50d650b6f0cfc
Gerrit-Change-Number: 17837
Gerrit-PatchSet: 8
Gerrit-Owner: Yingchun Lai 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Yingchun Lai 
Gerrit-Comment-Date: Tue, 14 Sep 2021 07:09:23 +
Gerrit-HasComments: No