쿼리 복잡도 분석 #13
쿼리 복잡도 분석 #13
Conversation
| @@ -76,7 +84,7 @@ | |||
| {:keys [timeout-ms timeout-error] | |||
| :or {timeout-ms 0 | |||
| timeout-error {:message "Query execution timed out."}}} options | |||
| execution-result (execute-parsed-query-async parsed-query variables context) | |||
| execution-result (execute-parsed-query-async parsed-query variables context options) | |||
There was a problem hiding this comment.
현재는 여기서 options에 :max-complexity 라는 key에 value를 넣어서 최대 리소스를 제한하는 것을 고려하였는데요.
system.edn 의 config 에 max-complexity를 하나 정의하여 context로 부터 가져와도 될 것 같다고 생각하는데 고민이긴 합니다.
There was a problem hiding this comment.
저희 백엔드 코드에서 build-lacinia-options할 때 현재 환경이 dev / staging인 경우에만 :max-complexity 값을 하드코딩해서 넘겨도 충분할 것 같습니다(ex: :max-complexity 1000000)
굳이 system.edn에는 추가 안 해도 될 것 같슴다
https://github.com/green-labs/farmmorning-backend/blob/68978493665d1027f45c4a64100a0855d5cb7781/bases/core-api/src/farmmorning/core_api/graphql/handler.clj#L97
다른 lacinia 옵션값들도 그냥 하드코딩하고 있음
There was a problem hiding this comment.
근데 지금 바로 반영된다면, 앱 코드들에 first: 9999 이런 잔재 때문에 프론트 개발자분들이 무조건 에러 응답들을 받아보실 텐데요..
우리 백엔드 미들웨어에서 라시니아 내부에서 뱉는 max-complexity 관련 에러는 로깅이나 슬랙만 보내고 by pass 한다던지의 의사결정만 될 듯 합니다!
There was a problem hiding this comment.
한 10만 정도로 설정해두고,
error에 code 하나 심어서 해당 code가 에러에 잡히면 근재님께서 제안해준 것 처럼 로깅이나 슬랙만 보내고 by pass하는게 좋을거같네요!
src/com/walmartlabs/lacinia.clj
Outdated
| :let [complexity-error (complexity-analysis/complexity-analysis prepared options)] | ||
|
|
||
| (some? complexity-error) | ||
| (resolve/resolve-as {:errors complexity-error}) |
There was a problem hiding this comment.
complexity-analysis 도 validator 중 하나로 설정할 수 있다면 좋겠는데...
이건 나중에 메인테이너가 어떻게 생각하는지 들어봅시다 ㅎㅎ
|
namenu
force-pushed
the
complexity-analysis
branch
from
f8a3e9e
to
f95f096
Compare
|
master 가 꼬여있어서 이 브랜치도 리베이스 했습니다. |
|
@namenu 미루고 미루다 드디어 테스트 케이스 하나 추가해두었습니당... |
| :n-nodes n-nodes}])))) | ||
|
|
||
| (defn ^:private calculate-complexity | ||
| [{:keys [selections list-args? n-nodes]}] |
There was a problem hiding this comment.
혹시 여기서 (seq selections) 체크가 필요한 것은 아닐지요...
There was a problem hiding this comment.
재귀 돌면서 leaf 가 selection이 없을 때의 동작이 정의되어 있지 않아 보여서요
수정: 꼭 leaf일 필요는 없겠네요
기존 팜모닝 코드 내에 존재하던 resource_limitation을 라시니아 내부로 이동시킵니다.
다른 언어로 작성된 라이브러리나 프레임워크들을 참고하였는데 (elixir - absinthe, ruby - graphql_ruby, haskell - hasura),
query validation -> (쿼리 복잡도 분석) -> execute 의 형태를 따라가고 있어 저희도 그와 같은 방식을 채택하여 작업하였습니다.
기존 팜모닝에서 작업했던 PR 에서는 단순히 request로 받은 쿼리를 분석하였다면, 이번에는 저희 schema와 쿼리를 먼저 합친 후 분석을 하도록 수정하였습니다.
그에 따라 schema에서 default로 정의된 값들을 쿼리 복잡도 분석에 사용하여 조금 더 정확한 분석결과를 얻을 수 있게되었습니다.