Permalink
Browse files

Add pool code back for review

This reverts commit a68535d.
  • Loading branch information...
roji committed Mar 22, 2018
1 parent a68535d commit 45e33ecef21f75f51a625c7b919a50da3ed8e920
Showing with 731 additions and 0 deletions.
  1. +587 −0 src/Npgsql/ConnectorPool.cs
  2. +144 −0 src/Npgsql/PoolManager.cs
Oops, something went wrong.

4 comments on commit 45e33ec

@Brar

This comment has been minimized.

Member

Brar replied Mar 22, 2018

Wow, I've learnt quite a bit just by reading the review comments.
👍

@roji

This comment has been minimized.

Member

roji replied Mar 30, 2018

To everyone following this, I've pushed 3 commits which should take care of all the above issues (f4276f4, 28d54ca, 10844c8).

A great thanks to @stephentoub for his help, and also to @FlorianRainer for the extra bugs uncovered. If anyone has the time and patience to take another look and make sure there aren't any more issues in there, that would be greatly appreciated.

@FlorianRainer

This comment has been minimized.

Contributor

FlorianRainer replied Apr 3, 2018

After a fast review the code seems to be fine, i will do some testing later, probably next week and i will keep you up to date.

The only thing, but its "only" a style thing, i saw the pattern below several times.
It may worth to create a Method with parameters for different state changes?

	var sw = new SpinWait();
	 while (true)
	 {
		state = State;
		state = State.Copy();
		 newState = state;
		 newState.Waiting--;
		 CheckInvariants(newState);
		 if (Interlocked.CompareExchange(ref State.All, newState.All, state.All) == state.All)
			 break;
		sw.SpinOnce();
	 }
@roji

This comment has been minimized.

Member

roji replied Apr 3, 2018

Thanks for the review. Any testing you can do would be really great, I'd like to feel as confident about this implementation as I can.

Regarding refactoring into a separate method, I thought about it at some point, but state changes sometimes involve changing one variable (Waiting in your example), others two... It seemed a bit like too much... but I may be wrong.

Please sign in to comment.