Skip to content
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

Netdb code cleanup #1949

Closed
wants to merge 3 commits into from
Closed

Conversation

fcrisciani
Copy link

This PR takes care of some code cleanup and is dependent from #1944

Flavio Crisciani added 3 commits September 21, 2017 09:37
- Added remainingReapTime field in the table event.
  Wihtout it a node that did not have a state for the element
  was marking the element for deletion setting the max reapTime.
  This was creating the possibility to keep the entry being resync
  between nodes forever avoding the purpose of the reap time
  itself.

- On broadcast of the table event the node owner was rewritten
  with the local node name, this was not correct because the owner
  should continue to remain the original one of the message

Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
- Changed the loop per network. Previous implementation was taking a
  ReadLock to update the reapTime but now with the residualReapTime
  also the bulkSync is using the same ReadLock creating possible
  issues in concurrent read and update of the value.
  The new logic fetches the list of networks and proceed to the
  cleanup network by network locking the database and releasing it
  after each network. This should ensure a fair locking avoiding
  to keep the database blocked for too much time.

  Note: The ticker does not guarantee that the reap logic runs
  precisely every reapTimePeriod, actually documentation says that
  if the routine is too long will skip ticks. In case of slowdown
  of the process itself it is possible that the lifetime of the
  deleted entries increases, it still should not be a huge problem
  because now the residual reaptime is propagated among all the nodes
  a slower node will let the deleted entry being repropagate multiple
  times but the state will still remain consistent.

Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
- Removed the reap network as is not super useful
- Adjusted some indentation

Signed-off-by: Flavio Crisciani <flavio.crisciani@docker.com>
@codecov-io
Copy link

Codecov Report

Merging #1949 into master will decrease coverage by 0.04%.
The diff coverage is 45.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1949      +/-   ##
==========================================
- Coverage      38%   37.95%   -0.05%     
==========================================
  Files         137      137              
  Lines       27149    27295     +146     
==========================================
+ Hits        10317    10360      +43     
- Misses      15562    15661      +99     
- Partials     1270     1274       +4
Impacted Files Coverage Δ
networkdb/broadcast.go 83.63% <100%> (+0.3%) ⬆️
networkdb/networkdb.go 59.26% <48.1%> (+2.25%) ⬆️
networkdb/networkdb.pb.go 37.31% <48.67%> (-1.54%) ⬇️
networkdb/delegate.go 70.17% <56.25%> (-0.67%) ⬇️
networkdb/cluster.go 47.01% <7.14%> (+0.88%) ⬆️
cmd/proxy/tcp_proxy.go 82.69% <0%> (+1.92%) ⬆️
endpoint_cnt.go 86.08% <0%> (+2.6%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c34e58a...4d5f193. Read the comment docs.

@fcrisciani fcrisciani closed this Sep 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants