[kudu-CR] Remove InMemoryEnv completely

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Remove InMemoryEnv completely
..


Patch Set 2: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4f26e321ec68a15b511db522e4d85470da7905f8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Remove InMemoryEnv completely

[Mike Percy (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Remove InMemoryEnv completely
..

Remove InMemoryEnv completely

Every time we extend the Env interface we need to create a mock
implementation for MemEnv. MemEnv really has no measurable benefit for
us. Let's delete some code!!!

Also removed a couple of unneeded #includes.

Change-Id: I4f26e321ec68a15b511db522e4d85470da7905f8
---
M build-support/release/rat_exclude_files.txt
M src/kudu/tablet/deltafile-test.cc
M src/kudu/util/CMakeLists.txt
D src/kudu/util/memenv/memenv-test.cc
D src/kudu/util/memenv/memenv.cc
D src/kudu/util/memenv/memenv.h
M src/kudu/util/rolling_log-test.cc
7 files changed, 0 insertions(+), 975 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/3258/2
-- 
To view, visit http://gerrit.cloudera.org:8080/3258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4f26e321ec68a15b511db522e4d85470da7905f8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Remove InMemoryEnv completely

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Remove InMemoryEnv completely
..


Patch Set 1:

Ah, should've used git grep.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4f26e321ec68a15b511db522e4d85470da7905f8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Remove InMemoryEnv completely

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Remove InMemoryEnv completely
..


Patch Set 1:

> There are no mentions of MemEnv or InMemoryEnv anywhere else in the code base 
> that I could find.

I found a couple more:

  
build-support/release/rat_exclude_files.txt:src/kudu/util/memenv/memenv-test.cc
  build-support/release/rat_exclude_files.txt:src/kudu/util/memenv/memenv.cc
  build-support/release/rat_exclude_files.txt:src/kudu/util/memenv/memenv.h

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4f26e321ec68a15b511db522e4d85470da7905f8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Remove InMemoryEnv completely

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: Remove InMemoryEnv completely
..


Patch Set 1:

There are no mentions of MemEnv or InMemoryEnv anywhere else in the code base 
that I could find.

For the EnvWrapper, it's not actually used by anything useful. However it does 
provide a mocking point that could be used when desired and the overhead of 
adding calls there when adding a new method is very low. So I'm inclined to 
leave it be.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4f26e321ec68a15b511db522e4d85470da7905f8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-HasComments: No

[kudu-CR] Remove InMemoryEnv completely

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Remove InMemoryEnv completely
..


Patch Set 1:

I think memenv would have been useful had we ever used it in non-trivial tests 
(i.e. tests that perform real I/O). But we never did, and like you said it's a 
tax on env changes, so I agree we should remove it.

Can you check the various license files to see if memenv is mentioned there?

Also, do we still need the env "wrapper" thing found at the bottom of env.h? Or 
is that also unnecessary now?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4f26e321ec68a15b511db522e4d85470da7905f8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Remove InMemoryEnv completely

[Mike Percy (Code Review)]

Hello Adar Dembo,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: Remove InMemoryEnv completely
..

Remove InMemoryEnv completely

Every time we extend the Env interface we need to create a mock
implementation for MemEnv. MemEnv really has no measurable benefit for
us. Let's delete some code!!!

Also removed a couple of unneeded #includes.

Change-Id: I4f26e321ec68a15b511db522e4d85470da7905f8
---
M src/kudu/tablet/deltafile-test.cc
M src/kudu/util/CMakeLists.txt
D src/kudu/util/memenv/memenv-test.cc
D src/kudu/util/memenv/memenv.cc
D src/kudu/util/memenv/memenv.h
M src/kudu/util/rolling_log-test.cc
6 files changed, 0 insertions(+), 972 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4f26e321ec68a15b511db522e4d85470da7905f8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Adar Dembo