You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2021/12/08 13:55:18 UTC

[GitHub] [camel-k] astefanutti opened a new pull request #2823: chore: Use atomic.Value for client-side apply fallback

astefanutti opened a new pull request #2823:
URL: https://github.com/apache/camel-k/pull/2823


   This PR follows up #2814 to take into account https://github.com/apache/camel-k/pull/2814#discussion_r764679710.
   
   @zregvart let me know if that corresponds to what you had in mind.
   
   **Release Note**
   ```release-note
   NONE
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-k] astefanutti edited a comment on pull request #2823: chore: Try SSA for Kamelet install only once

Posted by GitBox <gi...@apache.org>.
astefanutti edited a comment on pull request #2823:
URL: https://github.com/apache/camel-k/pull/2823#issuecomment-989772452


   @zregvart I've taken on your idea to use a mutex, and I've gone for using `sync.Once`, which encapsulates the mutex, and exposes just the right semantic for doing the _SSA attempt / fallback_ logic only once.
   
   Interestingly, the installation of the 137 bundled Kamelets now takes less than 200ms on my computer using Minikube, which is less than what I observed without the extra synchronisation. That being said, we're probably inside the measurement margin error, but at least the impact is either negligible or positive.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-k] astefanutti edited a comment on pull request #2823: chore: Use atomic.Value for client-side apply fallback

Posted by GitBox <gi...@apache.org>.
astefanutti edited a comment on pull request #2823:
URL: https://github.com/apache/camel-k/pull/2823#issuecomment-989772452


   @zregvart I've taken on your idea to use a mutex, and I've gone for using `sync.Once`, which encapsulates the mutex, and exposes just the right semantic for doing the _SSA attempt / fallback_ logic only once.
   
   Interestingly, the installation of the 137 bundled Kamelets now takes less than 200ms on my computer using Minikube, which is less than what I observed without the extra synchronisation. That being said, we're probably inside the measurement margin error to draw any conclusion.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-k] astefanutti merged pull request #2823: chore: Try SSA for Kamelet install only once

Posted by GitBox <gi...@apache.org>.
astefanutti merged pull request #2823:
URL: https://github.com/apache/camel-k/pull/2823


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-k] astefanutti commented on pull request #2823: chore: Use atomic.Value for client-side apply fallback

Posted by GitBox <gi...@apache.org>.
astefanutti commented on pull request #2823:
URL: https://github.com/apache/camel-k/pull/2823#issuecomment-989772452


   @zregvart I've taken on your idea to use a mutex, and I've gone for using `sync.Once`, which encapsulates the mutex, and exposes just the right semantic for doing the _SSA attempt / fallback_ logic only once.
   
   Interestingly, the installation of the 137 bundled Kamelets now takes less than 200ms on my computer, which is less than what I observed without the extra synchronisation. That being said, we're probably inside the measurement margin error to draw any conclusion.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [camel-k] zregvart commented on a change in pull request #2823: chore: Use atomic.Value for client-side apply fallback

Posted by GitBox <gi...@apache.org>.
zregvart commented on a change in pull request #2823:
URL: https://github.com/apache/camel-k/pull/2823#discussion_r765243801



##########
File path: pkg/install/kamelets.go
##########
@@ -50,7 +52,15 @@ const (
 	defaultKameletDir = "/kamelets/"
 )
 
-var hasServerSideApply = true
+var (
+	log = logf.Log
+
+	hasServerSideApply atomic.Value

Review comment:
       ```suggestion
   	hasServerSideApply atomic.Value
   	hasServerSideApplyWrite sync.Mutex
   ```

##########
File path: pkg/install/kamelets.go
##########
@@ -121,13 +131,14 @@ func applyKamelet(ctx context.Context, c client.Client, path string, namespace s
 	kamelet.GetLabels()[v1alpha1.KameletBundledLabel] = "true"
 	kamelet.GetLabels()[v1alpha1.KameletReadOnlyLabel] = "true"
 
-	if hasServerSideApply {
+	if v := hasServerSideApply.Load(); v.(bool) {

Review comment:
       ```suggestion
   	if v := hasServerSideApply.Load(); v == nil || v.(bool) {
   		if v == nil {
   			hasServerSideApplyWrite.Lock()
   			defer hasServerSideApplyWrite.Unlock()
   		}
   ```

##########
File path: pkg/install/kamelets.go
##########
@@ -121,13 +131,14 @@ func applyKamelet(ctx context.Context, c client.Client, path string, namespace s
 	kamelet.GetLabels()[v1alpha1.KameletBundledLabel] = "true"
 	kamelet.GetLabels()[v1alpha1.KameletReadOnlyLabel] = "true"
 
-	if hasServerSideApply {
+	if v := hasServerSideApply.Load(); v.(bool) {
 		err := serverSideApply(ctx, c, kamelet)
 		switch {
 		case err == nil:
 			return nil

Review comment:
       ```suggestion
   			if v == nil {
   				hasServerSideApply.Store(true)
   			}
   			return nil
   ```

##########
File path: pkg/install/kamelets.go
##########
@@ -27,6 +27,7 @@ import (
 	"path"
 	"path/filepath"
 	"strings"
+	"sync/atomic"

Review comment:
       ```suggestion
   	"sync/atomic"
   	"sync"
   ```

##########
File path: pkg/install/kamelets.go
##########
@@ -50,7 +52,15 @@ const (
 	defaultKameletDir = "/kamelets/"
 )
 
-var hasServerSideApply = true
+var (
+	log = logf.Log
+
+	hasServerSideApply atomic.Value
+)
+
+func init() {
+	hasServerSideApply.Store(true)
+}

Review comment:
       ```suggestion
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org