[GitHub] [yunikorn-core] pbacsko commented on a diff in pull request #507: [YUNIKORN-1385] To provide visibility to application's aggregated resource consumption

["pbacsko (via GitHub)" <gi...@apache.org>]

pbacsko commented on code in PR #507:
URL: https://github.com/apache/yunikorn-core/pull/507#discussion_r1193009065


##########
pkg/common/resources/resources.go:
##########
@@ -44,6 +46,59 @@ func (q Quantity) string() string {
 	return strconv.FormatInt(int64(q), 10)
 }
 
+// Util struct to keep track of application resource usage
+type UsedResource struct {
+	// Two level map for aggregated resource usage
+	// With instance type being the top level key, the mapped value is a map:
+	//   resource type (CPU, memory etc) -> the aggregated used time (in seconds) of the resource type
+	//
+	UsedResourceMap map[string]map[string]int64
+
+	sync.RWMutex
+}
+
+func (ur *UsedResource) Clone() *UsedResource {
+	if ur == nil {
+		return nil
+	}
+	ret := NewUsedResource()
+	ur.RLock()
+	defer ur.RUnlock()
+	for k, v := range ur.UsedResourceMap {
+		sourceEntry := map[string]int64(v)

Review Comment:
   If I'm reading this correctly, this is completely unnecessary and very complicated read. The variable itself and the type conversion too. Just range through "v". 



-- 
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: reviews-unsubscribe@yunikorn.apache.org

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