[GitHub] [incubator-yunikorn-core] yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API to retrieve app/container history info
yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API
to retrieve app/container history info
URL:
https://github.com/apache/incubator-yunikorn-core/pull/109#discussion_r398248259
##
File path: pkg/cache/history/history_info.go
##
@@ -0,0 +1,56 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package history
+
+import (
+ "time"
+)
+
+var LIMIT = 144 // 10 minutes in 1 day
+
+type HistoricalPartitionInfo struct {
+ records []*PartitionInfoRecord
Review comment:
you will need a lock for this.
As you might update the records and the same time the rest server is
retrieving the values.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@yunikorn.apache.org
For additional commands, e-mail: dev-h...@yunikorn.apache.org
[GitHub] [incubator-yunikorn-core] yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API to retrieve app/container history info
yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API to retrieve app/container history info URL: https://github.com/apache/incubator-yunikorn-core/pull/109#discussion_r398252203 ## File path: pkg/cache/history/history_info.go ## @@ -0,0 +1,56 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package history + +import ( + "time" +) + +var LIMIT = 144 // 10 minutes in 1 day Review comment: 10 minutes will be too long. can we use a 1 minute interval, and keep 12 hours data? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@yunikorn.apache.org For additional commands, e-mail: dev-h...@yunikorn.apache.org
[GitHub] [incubator-yunikorn-core] yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API to retrieve app/container history info
yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API
to retrieve app/container history info
URL:
https://github.com/apache/incubator-yunikorn-core/pull/109#discussion_r398249521
##
File path: pkg/cache/history/history_info.go
##
@@ -0,0 +1,56 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package history
+
+import (
+ "time"
+)
+
+var LIMIT = 144 // 10 minutes in 1 day
+
+type HistoricalPartitionInfo struct {
+ records []*PartitionInfoRecord
+}
+
+type PartitionInfoRecord struct {
+ Timestamp time.Time
+ TotalApplications int
+ TotalContainers int
+}
+
+func (hpInfo *HistoricalPartitionInfo) Store(totalApplications,
totalContainers int) {
Review comment:
let's keep the ref short, `hpInfo` -> `h`
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@yunikorn.apache.org
For additional commands, e-mail: dev-h...@yunikorn.apache.org
[GitHub] [incubator-yunikorn-core] yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API to retrieve app/container history info
yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API to retrieve app/container history info URL: https://github.com/apache/incubator-yunikorn-core/pull/109#discussion_r398252947 ## File path: pkg/cache/history_collector.go ## @@ -0,0 +1,76 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package cache + +import ( + "time" + + "github.com/apache/incubator-yunikorn-core/pkg/cache/history" + "github.com/apache/incubator-yunikorn-core/pkg/log" +) + +var tickerDefault = 10 * time.Minute Review comment: change to 1 minute 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@yunikorn.apache.org For additional commands, e-mail: dev-h...@yunikorn.apache.org
[GitHub] [incubator-yunikorn-core] yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API to retrieve app/container history info
yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API
to retrieve app/container history info
URL:
https://github.com/apache/incubator-yunikorn-core/pull/109#discussion_r398258503
##
File path: pkg/cache/partition_info.go
##
@@ -103,6 +106,11 @@ func newPartitionInfo(partition configs.PartitionConfig,
rmID string, info *Clus
// TODO get the resolver from the config
p.userGroupCache = security.GetUserGroupCache("")
+ hpInfo := {}
+ p.history = hpInfo
+ p.historyUpdater = NewHistoricalPartitionInfoUpdater(p, hpInfo)
+ p.historyUpdater.Start()
+
Review comment:
Can we move this to `cluster_info.StartService()` ?
Having a Start call in the `newPartitionInfo()` seems not appropriate.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@yunikorn.apache.org
For additional commands, e-mail: dev-h...@yunikorn.apache.org
[GitHub] [incubator-yunikorn-core] yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API to retrieve app/container history info
yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API
to retrieve app/container history info
URL:
https://github.com/apache/incubator-yunikorn-core/pull/109#discussion_r398270209
##
File path: pkg/webservice/handlers.go
##
@@ -235,3 +236,57 @@ func getNodeJSON(nodeInfo *cache.NodeInfo)
*dao.NodeDAOInfo {
Schedulable: nodeInfo.IsSchedulable(),
}
}
+
+func GetApplicationHistory(w http.ResponseWriter, r *http.Request) {
+ writeHeaders(w)
+
+ var result []*dao.ApplicationHistoryDAOInfo
+ lists := gClusterInfo.ListPartitions()
+ for _, k := range lists {
Review comment:
right now it reports back per partition
I don't think front-end expect this. we should aggregate this to the cluster
level. Can we change the `HistoricPartitionInfo` to `HistoricClusterInfo`?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@yunikorn.apache.org
For additional commands, e-mail: dev-h...@yunikorn.apache.org
[GitHub] [incubator-yunikorn-core] yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API to retrieve app/container history info
yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API to retrieve app/container history info URL: https://github.com/apache/incubator-yunikorn-core/pull/109#discussion_r398251737 ## File path: pkg/cache/history/history_info.go ## @@ -0,0 +1,56 @@ +/* + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package history + +import ( + "time" +) + +var LIMIT = 144 // 10 minutes in 1 day Review comment: can we add a construct method for this, with 1 parameter about the size? so we can remove `setLimitForTest()`, and just construct different objects with different passing size. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: dev-unsubscr...@yunikorn.apache.org For additional commands, e-mail: dev-h...@yunikorn.apache.org
[GitHub] [incubator-yunikorn-core] yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API to retrieve app/container history info
yangwwei commented on a change in pull request #109: [YUNIKORN-14] Add rest API
to retrieve app/container history info
URL:
https://github.com/apache/incubator-yunikorn-core/pull/109#discussion_r398253371
##
File path: pkg/cache/history_collector.go
##
@@ -0,0 +1,76 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+*/
+
+package cache
+
+import (
+ "time"
+
+ "github.com/apache/incubator-yunikorn-core/pkg/cache/history"
+ "github.com/apache/incubator-yunikorn-core/pkg/log"
+)
+
+var tickerDefault = 10 * time.Minute
+
+type HistoryProvider interface {
+ GetTotalApplicationCount() int
+ GetTotalAllocationCount() int
+}
+
+type HistoricalPartitionInfoUpdater struct {
+ finished chan bool
+ ticker *time.Ticker
+ pInfoHistoryProvider
+ hpInfo *history.HistoricalPartitionInfo
+}
+
+func NewHistoricalPartitionInfoUpdater(pInfo HistoryProvider, hpInfo
*history.HistoricalPartitionInfo) *HistoricalPartitionInfoUpdater {
+ finished := make(chan bool)
+ ticker := time.NewTicker(tickerDefault)
+
+ return {
+ finished,
+ ticker,
+ pInfo,
+ hpInfo,
+ }
+}
+
+func (hpInfoUpdater *HistoricalPartitionInfoUpdater) Start() {
+ go func() {
+ for {
+ select {
+ case <-hpInfoUpdater.finished:
+ return
+ case <-hpInfoUpdater.ticker.C:
+ log.Logger().Info("Adding current status to
historical partition data")
Review comment:
`INFO` to `DEBUG`
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: dev-unsubscr...@yunikorn.apache.org
For additional commands, e-mail: dev-h...@yunikorn.apache.org
