From d14723f24a8a45005a267ff5b69fe6df77605a7e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 3 Oct 2017 16:16:21 +0100 Subject: [PATCH 1/5] add rate limits for autocompiles global rate limit for all users and a lower rate limit for free users --- .../Features/Compile/CompileManager.coffee | 46 +++++++++++++++---- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/CompileManager.coffee b/services/web/app/coffee/Features/Compile/CompileManager.coffee index 61fe7038a6..756222121a 100755 --- a/services/web/app/coffee/Features/Compile/CompileManager.coffee +++ b/services/web/app/coffee/Features/Compile/CompileManager.coffee @@ -34,12 +34,16 @@ module.exports = CompileManager = return callback(error) if error? for key, value of limits options[key] = value - # only pass user_id down to clsi if this is a per-user compile - compileAsUser = if Settings.disablePerUserCompiles then undefined else user_id - ClsiManager.sendRequest project_id, compileAsUser, options, (error, status, outputFiles, clsiServerId, validationProblems) -> - return callback(error) if error? - logger.log files: outputFiles, "output files" - callback(null, status, outputFiles, clsiServerId, limits, validationProblems) + # Put a lower limit on autocompiles for free users + CompileManager._checkIfFreeAutoCompileLimitHasBeenHit options.isAutoCompile, limits.compileGroup, (err, canCompile)-> + if !canCompile + return callback null, "autocompile-backoff", [] + # only pass user_id down to clsi if this is a per-user compile + compileAsUser = if Settings.disablePerUserCompiles then undefined else user_id + ClsiManager.sendRequest project_id, compileAsUser, options, (error, status, outputFiles, clsiServerId, validationProblems) -> + return callback(error) if error? + logger.log files: outputFiles, "output files" + callback(null, status, outputFiles, clsiServerId, limits, validationProblems) stopCompile: (project_id, user_id, callback = (error) ->) -> @@ -75,15 +79,41 @@ module.exports = CompileManager = _checkIfAutoCompileLimitHasBeenHit: (isAutoCompile, callback = (err, canCompile)->)-> if !isAutoCompile return callback(null, true) - opts = + Metrics.inc "auto-compile" + opts = endpointName:"auto_compile" timeInterval:20 subjectName:"everyone" - throttle: 25 + throttle: 200 rateLimiter.addCount opts, (err, canCompile)-> if err? canCompile = false logger.log canCompile:canCompile, opts:opts, "checking if auto compile limit has been hit" + if !canCompile + Metrics.inc "auto-compile-rate-limited" + callback err, canCompile + + + _checkIfFreeAutoCompileLimitHasBeenHit: (isAutoCompile, compileGroup, callback = (err, canCompile)->)-> + if !isAutoCompile + return callback(null, true) + + if compileGroup is "priority" + Metrics.inc "auto-compile-priority" + return callback(null, true) + + Metrics.inc "auto-compile-free" + opts = + endpointName:"auto_compile" + timeInterval:20 + subjectName:"free" + throttle: 100 + rateLimiter.addCount opts, (err, canCompile)-> + if err? + canCompile = false + logger.log canCompile:canCompile, opts:opts, "checking if free users auto compile limit has been hit" + if !canCompile + Metrics.inc "auto-compile-free-rate-limited" callback err, canCompile _ensureRootDocumentIsSet: (project_id, callback = (error) ->) -> From 2723537f824d757fa07f61bc1892571357c73d69 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Tue, 3 Oct 2017 16:23:49 +0100 Subject: [PATCH 2/5] disable autocompile when rate limit is hit --- services/web/app/views/project/editor/pdf.pug | 6 ++++++ .../coffee/ide/pdf/controllers/PdfController.coffee | 12 ++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/services/web/app/views/project/editor/pdf.pug b/services/web/app/views/project/editor/pdf.pug index 377f09f71a..02531e9584 100644 --- a/services/web/app/views/project/editor/pdf.pug +++ b/services/web/app/views/project/editor/pdf.pug @@ -391,6 +391,12 @@ div.full-size.pdf(ng-controller="PdfController") ng-click="startFreeTrial('compile-timeout')" ) #{translate("start_free_trial")} + + .alert.alert-danger(ng-show="pdf.autocompile_disabled") + p + strong #{translate("autocompile_disabled")}. + span #{translate("autocompile_disabled_reason")} + .alert.alert-danger(ng-show="pdf.projectTooLarge") strong #{translate("project_too_large")} span #{translate("project_too_large_please_reduce")} diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index 73ae9a4380..e77d4a22cb 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -127,7 +127,7 @@ define [ sendCompileRequest = (options = {}) -> url = "/project/#{$scope.project_id}/compile" params = {} - if options.isAutoCompile + if options.isAutoCompile or options.isBackgroundAutoCompile params["auto_compile"]=true # if the previous run was a check, clear the error logs $scope.pdf.logEntries = [] if $scope.check @@ -168,6 +168,7 @@ define [ $scope.pdf.compileExited = false $scope.pdf.failedCheck = false $scope.pdf.compileInProgress = false + $scope.pdf.autocompile_disabled = false # make a cache to look up files by name fileByPath = {} @@ -206,7 +207,13 @@ define [ $scope.shouldShowLogs = true fetchLogs(fileByPath) else if response.status == "autocompile-backoff" - $scope.pdf.view = 'uncompiled' + if $scope.pdf.isAutoCompile # initial autocompile + $scope.pdf.view = 'uncompiled' + else # background autocompile from typing + $scope.pdf.view = 'errors' + $scope.pdf.autocompile_disabled = true + $scope.autocompile_enabled = false # disable any further autocompiles + event_tracking.sendMB "autocompile-rate-limited", {hasPremiumCompile: $scope.hasPremiumCompile} else if response.status == "project-too-large" $scope.pdf.view = 'errors' $scope.pdf.projectTooLarge = true @@ -415,6 +422,7 @@ define [ event_tracking.sendMBSampled "editor-recompile-sampled", options $scope.pdf.compiling = true + $scope.pdf.isAutoCompile = options?.isAutoCompile # initial autocompile if options?.force # for forced compile, turn off validation check and ignore errors From 15e2deed7321a1edcb3410a5d317cb25a0f143a2 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 9 Oct 2017 15:18:55 +0100 Subject: [PATCH 3/5] rename isAutoCompile and isBackgroundAutoCompile changed to isAutoCompileOnLoad and isAutoCompileOnChange --- .../coffee/ide/pdf/controllers/PdfController.coffee | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index e77d4a22cb..bb27057fc7 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -68,7 +68,7 @@ define [ $scope.$on "project:joined", () -> return if !autoCompile autoCompile = false - $scope.recompile(isAutoCompile: true) + $scope.recompile(isAutoCompileOnLoad: true) $scope.hasPremiumCompile = $scope.project.features.compileGroup == "priority" $scope.$on "pdf:error:display", () -> @@ -86,7 +86,7 @@ define [ if isTimeNonMonotonic || timeSinceLastCompile >= AUTO_COMPILE_TIMEOUT if (!ide.$scope.hasLintingError) - $scope.recompile(isBackgroundAutoCompile: true) + $scope.recompile(isAutoCompileOnChange: true) else # Extend remainder of timeout autoCompileTimeout = setTimeout () -> @@ -127,7 +127,7 @@ define [ sendCompileRequest = (options = {}) -> url = "/project/#{$scope.project_id}/compile" params = {} - if options.isAutoCompile or options.isBackgroundAutoCompile + if options.isAutoCompileOnLoad or options.isAutoCompileOnChange params["auto_compile"]=true # if the previous run was a check, clear the error logs $scope.pdf.logEntries = [] if $scope.check @@ -207,7 +207,7 @@ define [ $scope.shouldShowLogs = true fetchLogs(fileByPath) else if response.status == "autocompile-backoff" - if $scope.pdf.isAutoCompile # initial autocompile + if $scope.pdf.isAutoCompileOnLoad # initial autocompile $scope.pdf.view = 'uncompiled' else # background autocompile from typing $scope.pdf.view = 'errors' @@ -422,7 +422,7 @@ define [ event_tracking.sendMBSampled "editor-recompile-sampled", options $scope.pdf.compiling = true - $scope.pdf.isAutoCompile = options?.isAutoCompile # initial autocompile + $scope.pdf.isAutoCompileOnLoad = options?.isAutoCompileOnLoad # initial autocompile if options?.force # for forced compile, turn off validation check and ignore errors From ea896380106b51c6ef45f401fb9f314d8fca28b9 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 9 Oct 2017 15:21:01 +0100 Subject: [PATCH 4/5] rename autocompile_disabled to autoCompileDisabled for consistency --- services/web/app/views/project/editor/pdf.pug | 2 +- .../public/coffee/ide/pdf/controllers/PdfController.coffee | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/app/views/project/editor/pdf.pug b/services/web/app/views/project/editor/pdf.pug index 02531e9584..f48171c7d0 100644 --- a/services/web/app/views/project/editor/pdf.pug +++ b/services/web/app/views/project/editor/pdf.pug @@ -392,7 +392,7 @@ div.full-size.pdf(ng-controller="PdfController") ) #{translate("start_free_trial")} - .alert.alert-danger(ng-show="pdf.autocompile_disabled") + .alert.alert-danger(ng-show="pdf.autoCompileDisabled") p strong #{translate("autocompile_disabled")}. span #{translate("autocompile_disabled_reason")} diff --git a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee index bb27057fc7..2eb1a59eb2 100644 --- a/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee +++ b/services/web/public/coffee/ide/pdf/controllers/PdfController.coffee @@ -168,7 +168,7 @@ define [ $scope.pdf.compileExited = false $scope.pdf.failedCheck = false $scope.pdf.compileInProgress = false - $scope.pdf.autocompile_disabled = false + $scope.pdf.autoCompileDisabled = false # make a cache to look up files by name fileByPath = {} @@ -211,7 +211,7 @@ define [ $scope.pdf.view = 'uncompiled' else # background autocompile from typing $scope.pdf.view = 'errors' - $scope.pdf.autocompile_disabled = true + $scope.pdf.autoCompileDisabled = true $scope.autocompile_enabled = false # disable any further autocompiles event_tracking.sendMB "autocompile-rate-limited", {hasPremiumCompile: $scope.hasPremiumCompile} else if response.status == "project-too-large" From 5b0d3d1429cfc369bd6bcd1d0f21ac8f46645c7f Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 9 Oct 2017 16:31:01 +0100 Subject: [PATCH 5/5] simplify rate-limit checking code --- .../Features/Compile/CompileManager.coffee | 47 ++++++------------- services/web/config/settings.defaults.coffee | 5 ++ .../coffee/Compile/CompileManagerTests.coffee | 16 +++---- 3 files changed, 28 insertions(+), 40 deletions(-) diff --git a/services/web/app/coffee/Features/Compile/CompileManager.coffee b/services/web/app/coffee/Features/Compile/CompileManager.coffee index 756222121a..9e50b280e2 100755 --- a/services/web/app/coffee/Features/Compile/CompileManager.coffee +++ b/services/web/app/coffee/Features/Compile/CompileManager.coffee @@ -18,7 +18,7 @@ module.exports = CompileManager = timer.done() _callback(args...) - @_checkIfAutoCompileLimitHasBeenHit options.isAutoCompile, (err, canCompile)-> + @_checkIfAutoCompileLimitHasBeenHit options.isAutoCompile, "everyone", (err, canCompile)-> if !canCompile return callback null, "autocompile-backoff", [] logger.log project_id: project_id, user_id: user_id, "compiling project" @@ -34,8 +34,8 @@ module.exports = CompileManager = return callback(error) if error? for key, value of limits options[key] = value - # Put a lower limit on autocompiles for free users - CompileManager._checkIfFreeAutoCompileLimitHasBeenHit options.isAutoCompile, limits.compileGroup, (err, canCompile)-> + # Put a lower limit on autocompiles for free users, based on compileGroup + CompileManager._checkCompileGroupAutoCompileLimit options.isAutoCompile, limits.compileGroup, (err, canCompile)-> if !canCompile return callback null, "autocompile-backoff", [] # only pass user_id down to clsi if this is a per-user compile @@ -76,44 +76,27 @@ module.exports = CompileManager = else return callback null, true - _checkIfAutoCompileLimitHasBeenHit: (isAutoCompile, callback = (err, canCompile)->)-> + _checkCompileGroupAutoCompileLimit: (isAutoCompile, compileGroup, callback = (err, canCompile)->)-> + if compileGroup is "default" + CompileManager._checkIfAutoCompileLimitHasBeenHit isAutoCompile, compileGroup, callback + else + Metrics.inc "auto-compile-#{compileGroup}" + return callback(null, true) # always allow priority group users to compile + + _checkIfAutoCompileLimitHasBeenHit: (isAutoCompile, compileGroup, callback = (err, canCompile)->)-> if !isAutoCompile return callback(null, true) - Metrics.inc "auto-compile" + Metrics.inc "auto-compile-#{compileGroup}" opts = endpointName:"auto_compile" timeInterval:20 - subjectName:"everyone" - throttle: 200 + subjectName:compileGroup + throttle: Settings?.rateLimit?.autoCompile?[compileGroup] || 25 rateLimiter.addCount opts, (err, canCompile)-> if err? canCompile = false - logger.log canCompile:canCompile, opts:opts, "checking if auto compile limit has been hit" if !canCompile - Metrics.inc "auto-compile-rate-limited" - callback err, canCompile - - - _checkIfFreeAutoCompileLimitHasBeenHit: (isAutoCompile, compileGroup, callback = (err, canCompile)->)-> - if !isAutoCompile - return callback(null, true) - - if compileGroup is "priority" - Metrics.inc "auto-compile-priority" - return callback(null, true) - - Metrics.inc "auto-compile-free" - opts = - endpointName:"auto_compile" - timeInterval:20 - subjectName:"free" - throttle: 100 - rateLimiter.addCount opts, (err, canCompile)-> - if err? - canCompile = false - logger.log canCompile:canCompile, opts:opts, "checking if free users auto compile limit has been hit" - if !canCompile - Metrics.inc "auto-compile-free-rate-limited" + Metrics.inc "auto-compile-#{compileGroup}-limited" callback err, canCompile _ensureRootDocumentIsSet: (project_id, callback = (error) ->) -> diff --git a/services/web/config/settings.defaults.coffee b/services/web/config/settings.defaults.coffee index 2456590709..94e9ae5007 100644 --- a/services/web/config/settings.defaults.coffee +++ b/services/web/config/settings.defaults.coffee @@ -437,3 +437,8 @@ module.exports = settings = # name : "all projects", # url: "/templates/all" #}] + + rateLimits: + autoCompile: + everyone: 100 + standard: 25 diff --git a/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee b/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee index 21327a50c9..195da8b850 100644 --- a/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Compile/CompileManagerTests.coffee @@ -44,7 +44,7 @@ describe "CompileManager", -> describe "succesfully", -> beforeEach -> - @CompileManager._checkIfAutoCompileLimitHasBeenHit = (_, cb)-> cb(null, true) + @CompileManager._checkIfAutoCompileLimitHasBeenHit = (isAutoCompile, compileGroup, cb)-> cb(null, true) @CompileManager.compile @project_id, @user_id, {}, @callback it "should check the project has not been recently compiled", -> @@ -84,7 +84,7 @@ describe "CompileManager", -> describe "when the project has been recently compiled", -> it "should return", (done)-> - @CompileManager._checkIfAutoCompileLimitHasBeenHit = (_, cb)-> cb(null, true) + @CompileManager._checkIfAutoCompileLimitHasBeenHit = (isAutoCompile, compileGroup, cb)-> cb(null, true) @CompileManager._checkIfRecentlyCompiled = sinon.stub().callsArgWith(2, null, true) @CompileManager.compile @project_id, @user_id, {}, (err, status)-> status.should.equal "too-recently-compiled" @@ -92,7 +92,7 @@ describe "CompileManager", -> describe "should check the rate limit", -> it "should return", (done)-> - @CompileManager._checkIfAutoCompileLimitHasBeenHit = sinon.stub().callsArgWith(1, null, false) + @CompileManager._checkIfAutoCompileLimitHasBeenHit = sinon.stub().callsArgWith(2, null, false) @CompileManager.compile @project_id, @user_id, {}, (err, status)-> status.should.equal "autocompile-backoff" done() @@ -222,14 +222,14 @@ describe "CompileManager", -> describe "_checkIfAutoCompileLimitHasBeenHit", -> it "should be able to compile if it is not an autocompile", (done)-> - @ratelimiter.addCount.callsArgWith(1, null, true) - @CompileManager._checkIfAutoCompileLimitHasBeenHit false, (err, canCompile)=> + @ratelimiter.addCount.callsArgWith(2, null, true) + @CompileManager._checkIfAutoCompileLimitHasBeenHit false, "everyone", (err, canCompile)=> canCompile.should.equal true done() it "should be able to compile if rate limit has remianing", (done)-> @ratelimiter.addCount.callsArgWith(1, null, true) - @CompileManager._checkIfAutoCompileLimitHasBeenHit true, (err, canCompile)=> + @CompileManager._checkIfAutoCompileLimitHasBeenHit true, "everyone", (err, canCompile)=> args = @ratelimiter.addCount.args[0][0] args.throttle.should.equal 25 args.subjectName.should.equal "everyone" @@ -240,13 +240,13 @@ describe "CompileManager", -> it "should be not able to compile if rate limit has no remianing", (done)-> @ratelimiter.addCount.callsArgWith(1, null, false) - @CompileManager._checkIfAutoCompileLimitHasBeenHit true, (err, canCompile)=> + @CompileManager._checkIfAutoCompileLimitHasBeenHit true, "everyone", (err, canCompile)=> canCompile.should.equal false done() it "should return false if there is an error in the rate limit", (done)-> @ratelimiter.addCount.callsArgWith(1, "error") - @CompileManager._checkIfAutoCompileLimitHasBeenHit true, (err, canCompile)=> + @CompileManager._checkIfAutoCompileLimitHasBeenHit true, "everyone", (err, canCompile)=> canCompile.should.equal false done()