[kudu-CR] tablet copy: Refactor to avoid separate expiration map

["Mike Percy (Code Review)" <ge...@cloudera.org>]

Mike Percy has posted comments on this change.

Change subject: tablet copy: Refactor to avoid separate expiration map
......................................................................


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/6731/1/src/kudu/tserver/tablet_copy_service.cc
File src/kudu/tserver/tablet_copy_service.cc:

Line 122:     const auto& iter = sessions_.find(session_id);
> Couldn't you still use FindCopy() as long as the third argument was a Sessi
Switched to FindOrNull() since FindCopy() makes a copy of the scoped_refptr<TabletCopySourceSession> which means we have to std::move() it below to avoid a double ref. Since std::move() can be prone to human error down the road I'd rather avoid it.


Line 134:       InsertOrDie(&sessions_, session_id, { .session = session, .expires = GetNewExpireTime() });
> I thought this was illegal in C++11:
It seems to work but it's apparently non-standard, so I'll remove it.


PS1, Line 261:   for (const auto& entry : sessions_) {
             :     session_ids.push_back(entry.first);
             :   }
             :   for (const string& session_id : session_ids) {
> Can these loops be merged?
Done


Line 276:   const auto& entry = sessions_.find(session_id);
> Here too.
Same as above


Line 319:   auto iter = sessions_.find(session_id);
> Here maybe FindOrNull, so you can get a pointer to the SessionEntry in orde
Sounds good, done


http://gerrit.cloudera.org:8080/#/c/6731/1/src/kudu/tserver/tablet_copy_service.h
File src/kudu/tserver/tablet_copy_service.h:

Line 49: struct SessionEntry {
> Maybe declare this as a nested and private member of TabletCopyServiceImpl,
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idcf4e57722e4e34e99064713a6e8d246d3ebf920
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes