Browse Source

Use better error messages instead of ErrUnavailable

When a session is closed it should not be possible to execute queries,
the error message for this is distinct to a live session which has
not connections.

If a live session has no available connections then indicate that
instead of the generic ErrUnavailable.
Chris Bannister 11 years ago
parent
commit
ef525ee05c
4 changed files with 61 additions and 26 deletions
  1. 3 3
      cassandra_test.go
  2. 2 2
      cluster.go
  3. 2 2
      conn_test.go
  4. 54 19
      session.go

+ 3 - 3
cassandra_test.go

@@ -320,10 +320,10 @@ func TestCreateSessionTimeout(t *testing.T) {
 	_, err := c.CreateSession()
 
 	if err == nil {
-		t.Fatal("expected ErrNoConncetions, but no error was returned.")
+		t.Fatal("expected ErrNoConnectionsStarted, but no error was returned.")
 	}
-	if err != ErrNoConnections {
-		t.Fatalf("expected ErrNoConnections, but received %v", err)
+	if err != ErrNoConnectionsStarted {
+		t.Fatalf("expected ErrNoConnectionsStarted, but received %v", err)
 	}
 }
 

+ 2 - 2
cluster.go

@@ -91,7 +91,7 @@ func (cfg *ClusterConfig) CreateSession() (*Session, error) {
 		return s, nil
 	}
 	impl.Close()
-	return nil, ErrNoConnections
+	return nil, ErrNoConnectionsStarted
 
 }
 
@@ -270,5 +270,5 @@ func (c *clusterImpl) Close() {
 
 var (
 	ErrNoHosts       = errors.New("no hosts provided")
-	ErrNoConnections = errors.New("no connections were made when creating the session")
+	ErrNoConnectionsStarted = errors.New("no connections were made when creating the session")
 )

+ 2 - 2
conn_test.go

@@ -46,8 +46,8 @@ func TestClosed(t *testing.T) {
 	}
 	session.Close()
 
-	if err := session.Query("void").Exec(); err != ErrUnavailable {
-		t.Errorf("expected %#v, got %#v", ErrUnavailable, err)
+	if err := session.Query("void").Exec(); err != ErrSessionClosed {
+		t.Errorf("expected %#v, got %#v", ErrSessionClosed, err)
 	}
 }
 

+ 54 - 19
session.go

@@ -31,6 +31,9 @@ type Session struct {
 	trace    Tracer
 	mu       sync.RWMutex
 	cfg      ClusterConfig
+
+	closeMu  sync.RWMutex
+	isClosed bool
 }
 
 // NewSession wraps an existing Node.
@@ -88,54 +91,84 @@ func (s *Session) Query(stmt string, values ...interface{}) *Query {
 // Close closes all connections. The session is unusable after this
 // operation.
 func (s *Session) Close() {
+	s.closeMu.Lock()
+	if s.isClosed {
+		s.closeMu.Unlock()
+		return
+	}
+	s.isClosed = true
+	s.closeMu.Unlock()
+
 	s.Node.Close()
 }
 
+func (s *Session) Closed() bool {
+	s.closeMu.RLock()
+	closed := s.isClosed
+	s.closeMu.RUnlock()
+	return closed
+}
+
 func (s *Session) executeQuery(qry *Query) *Iter {
-	var itr *Iter
-	count := 0
-	for count <= qry.rt.NumRetries {
-		conn := s.Node.Pick(nil)
+	// fail fast
+	if s.Closed() {
+		return &Iter{err: ErrSessionClosed}
+	}
+
+	var iter *Iter
+	for count := 0; count <= qry.rt.NumRetries; count++ {
+		conn := s.Node.Pick(qry)
 		//Assign the error unavailable to the iterator
 		if conn == nil {
-			itr = &Iter{err: ErrUnavailable}
+			iter = &Iter{err: ErrNoConnections}
 			break
 		}
-		itr = conn.executeQuery(qry)
+
+		iter = conn.executeQuery(qry)
 		//Exit for loop if the query was successful
-		if itr.err == nil {
+		if iter.err == nil {
 			break
 		}
-		count++
 	}
-	return itr
+
+	if iter == nil {
+		iter = &Iter{err: ErrNoConnections}
+	}
+
+	return iter
 }
 
 // ExecuteBatch executes a batch operation and returns nil if successful
 // otherwise an error is returned describing the failure.
 func (s *Session) ExecuteBatch(batch *Batch) error {
+	// fail fast
+	if s.Closed() {
+		return ErrSessionClosed
+	}
+
 	// Prevent the execution of the batch if greater than the limit
 	// Currently batches have a limit of 65536 queries.
 	// https://datastax-oss.atlassian.net/browse/JAVA-229
 	if batch.Size() > BatchSizeMaximum {
 		return ErrTooManyStmts
 	}
+
 	var err error
-	count := 0
-	for count <= batch.rt.NumRetries {
+	for count := 0; count <= batch.rt.NumRetries; count++ {
 		conn := s.Node.Pick(nil)
 		//Assign the error unavailable and break loop
 		if conn == nil {
-			err = ErrUnavailable
+			err = ErrNoConnections
 			break
 		}
+
 		err = conn.executeBatch(batch)
 		//Exit loop if operation executed correctly
 		if err == nil {
-			break
+			return nil
 		}
-		count++
 	}
+
 	return err
 }
 
@@ -489,11 +522,13 @@ func (e Error) Error() string {
 }
 
 var (
-	ErrNotFound     = errors.New("not found")
-	ErrUnavailable  = errors.New("unavailable")
-	ErrUnsupported  = errors.New("feature not supported")
-	ErrTooManyStmts = errors.New("too many statements")
-	ErrUseStmt      = errors.New("use statements aren't supported. Please see https://github.com/gocql/gocql for explaination.")
+	ErrNotFound      = errors.New("not found")
+	ErrUnavailable   = errors.New("unavailable")
+	ErrUnsupported   = errors.New("feature not supported")
+	ErrTooManyStmts  = errors.New("too many statements")
+	ErrUseStmt       = errors.New("use statements aren't supported. Please see https://github.com/gocql/gocql for explaination.")
+	ErrSessionClosed = errors.New("session has been closed")
+	ErrNoConnections = errors.New("no connections available")
 )
 
 type ErrProtocol struct{ error }