데이터베이스 리뷰 지침

이 페이지는 데이터베이스 리뷰에 특화되어 있습니다. 일반적인 코드 리뷰에 관한 넓은 조언과 최선의 방법에 대해서는 코드 리뷰 가이드를 참조하십시오.

일반적인 프로세스

데이터베이스 리뷰는 다음과 같은 경우에 필요합니다.

  • 데이터베이스 스키마를 건드리거나 데이터 이관을 수행하는 변경 사항,
    • db/
    • lib/gitlab/background_migration/ 내의 파일을 포함합니다.
  • 데이터베이스 도구에 대한 변경 사항. 예를 들어:
    • lib/gitlab/database/ 내의 마이그레이션 또는 ActiveRecord 도우미
    • 로드 밸런싱
  • 분명히 이해하기 어려운 SQL 쿼리를 생성하는 변경 사항. 일반적으로 병합 요청의 저자가 복잡한 쿼리가 도입되고 데이터베이스 리뷰가 필요한지 여부를 결정해야 합니다.
  • count, distinct_count, estimate_batch_distinct_count, sum을 사용하는 서비스 데이터 메트릭스의 변경 사항. 이러한 메트릭스에는 대형 테이블에 걸친 복잡한 쿼리가 있을 수 있습니다. 자세한 내용은 분석 계측 가이드를 참조하십시오.
  • ActiveRecord 객체에 대해 update, upsert, delete, update_all, upsert_all, delete_all, destroy_all 메서드를 사용하는 변경 사항.

데이터베이스 리뷰어는 변경 사항에서 지나치게 복잡한 쿼리를 주시하고 더 면밀하게 검토해야 합니다. 저자가 특정 쿼리를 검토할 것을 지목하지 않고 지나치게 복잡한 쿼리가 없는 경우에는 마이그레이션만을 검토하는 것으로 충분합니다.

필수사항

다음 항목들을 병합 요청시에 제공해야 합니다. 만약 병합 요청 설명에 이러한 항목이 포함되어 있지 않다면, 리뷰는 저자에게 재할당됩니다.

마이그레이션

새로운 마이그레이션이 도입된 경우, 데이터베이스 리뷰어는 모든 마이그레이션의 마이그레이션(db:migrate)과 롤백(db:rollback)의 출력을 검토해야 합니다.

우리는 GitLab에서 (제공되는 db:check-migrations 파이프라인 작업) 자동화된 도구를 가지고 있습니다. 이 도구는 CI 작업 로그에서 이러한 출력을 제공합니다. 저자가 병합 요청 설명에 이러한 출력을 제공할 필요는 없지만, 그렇게 하는 것이 리뷰어에게 도움이 될 수 있습니다. 봇은 또한 마이그레이션이 제대로 되돌릴 수 있는지 확인합니다.

쿼리

새로운 쿼리가 도입되거나 기존 쿼리가 업데이트된 경우, 다음을 제공해야 합니다:

  • 병합 요청에 포함된 모든 원시 SQL 쿼리의 쿼리 계획과 각 원시 SQL 스니펫 뒤를 따라가는 쿼리 계획에 대한 링크.
  • 변경되거나 추가된 모든 쿼리에 대한 원시 SQL(ActiveRecord 쿼리에서 변환된 형태).
    • 기존 쿼리를 업데이트하는 경우, 예전 버전과 새 버전의 쿼리의 원시 SQL과 그들의 쿼리 계획을 함께 제공해야 합니다.

이 정보를 제공하는 방법에 대해서는 쿼리 추가 또는 수정시 준비를 참조하십시오.

역할 및 프로세스

병합 요청 저자의 역할은 다음과 같습니다:

데이터베이스 리뷰어의 역할은 다음과 같습니다:

  • 필요한 artifacts가 제대로 제공되었는지 확인합니다. 그렇지 않다면, 병합 요청을 저자에게 재할당합니다.
  • 병합 요청에 대한 첫 번째 검토를 수행하고 저자에게 개선사항을 제안합니다.
  • 만족한 경우, 병합 요청에 ~"database::reviewed" 라벨을 다시 지정하고 승인하며 Reviewer Roulette가 추천한 데이터베이스 유지 관리자에게 검토를 요청합니다. 이 작업을 완료하면 리뷰어 자신을 제거합니다.

데이터베이스 유지 관리자의 역할은 다음과 같습니다:

  • 병합 요청에 대한 최종 데이터베이스 검토를 수행합니다.
  • 데이터베이스 리뷰어와 병합 요청의 저자와 추가적인 개선사항 또는 관련 변경에 대해 논의합니다.
  • 병합 요청을 최종 승인하고, 병합 요청에 ~"database::approved" 라벨을 다시 지정합니다.
  • 다른 승인이 대기 중이지 않다면, 병합 요청을 병합하거나 필요에 따라 다른 유지 관리자들에게 전달합니다(프론트엔드, 백엔드, 문서).
    • 병합하지 않는 경우, 리뷰어 자신을 제거합니다.

리뷰 작업 분산

리뷰어 룰렛을 사용하여 리뷰 작업을 분배합니다. (예시). 병합 요청자는 제안된 데이터베이스 리뷰어에게서 리뷰를 요청해야 합니다. 그들이 승인하면, 제안된 데이터베이스 유지 관리자에게 넘겨야 합니다.

리뷰어 룰렛이 데이터베이스 리뷰어 및 유지 관리자를 제안하지 않은 경우, ~database 라벨을 적용했는지 확인하고 danger-review CI 작업을 다시 실행하거나 @gl-database 팀에서 누군가를 선택하십시오.

데이터베이스 리뷰를 위한 병합 요청 준비 방법

검토를 더 쉽고 빠르게 하기 위해 다음 준비 사항들을 고려해주십시오.

마이그레이션 추가시 준비 사항

  • db/structure.sql이 문서화된대로 업데이트되었는지 확인하고 추가 또는 제거된 db/schema_migrations 하위 해당 버전 파일이 있는지 확인하십시오.
  • Database Dictionary가 문서화된대로 업데이트되었는지 확인하십시오.
  • change 메서드를 사용하거나 up를 사용할 때 down 메서드를 포함하여, 마이그레이션을 복구할 수 있도록 만들어보십시오.
    • 롤백 절차를 포함하거나 변경 사항을 롤백하는 방법에 대해 설명하십시오.
  • db:check-migrations 파이프라인 작업이 성공적으로 실행되었고, 마이그레이션 롤백이 예상대로 작동하는지 확인하십시오.
    • db:check-schema 작업이 성공적으로 실행되었고, 롤백시 예상치 못한 스키마 변경이 없는지 확인하십시오. 이 작업은 스키마가 변경된 경우에만 경고를 트리거할 수 있습니다.
    • 리뷰 프로세스 중 마이그레이션을 수정할 때마다 앞서 언급한 작업들이 계속해서 성공하는지 확인하십시오.
  • 필요한 경우 spec/migrations에 마이그레이션을 위한 테스트를 추가하십시오. 자세한 내용은 GitLab에서 Rails 마이그레이션 테스트를 참조하십시오.
  • 트랜잭션 마이그레이션에 대해서는 임계 재시도 기능이 기본으로 활성화됩니다. 트랜잭션과 무관한 마이그레이션의 경우, 사용 사례와 해결책에 대한 관련 문서를 검토하십시오.
  • 적절한 이유가 없다면 RuboCop 검사를 비활성화하지 않았는지 확인하십시오.
  • 대형 테이블에 인덱스를 추가하는 경우, Database Lab에서 CREATE INDEX CONCURRENTLY를 사용하여 실행을 테스트하고 실행 시간을 병합 요청 설명에 추가하십시오:
    • Database Lab의 실행 시간이 GitLab.com의 실행 시간도 상당히 길다는 힌트를 줄 수 있지만, Database Lab의 실행 시간이 10분보다 길다면, 인덱스는 배포 후 마이그레이션으로 이동해야 합니다. 이 경우, 해당 인덱스가 필요한 코드가 배포될 때 인덱스가 있도록 하기 위해 마이그레이션과 응용 프로그램 변경을 분리하여 별도의 릴리스로 나눌 필요가 있습니다.
  • test 단계에서 database testing 작업(db:gitlabcom-database-testing)을 수동으로 시작해주십시오.
    • 이 작업은 Database Lab의 클론에서 마이그레이션을 실행하고 그 결과(쿼리, 실행 시간, 크기의 변경)를 병합 요청에 게시합니다.
    • 마이그레이션 실행 시간과 경고를 검토하십시오.

데이터 마이그레이션 추가 시 준비 사항

데이터 마이그레이션은 본질적으로 위험합니다. 생산 데이터 손상 또는 손실 가능성을 줄이기 위해 추가 조치가 필요합니다.

MR(Merge Request) 설명에 포함:

  • 마이그레이션이 복구할 수 없는 경우, 사건 발생 시 데이터 변경을 복구할 수 있는 방법에 대한 세부 정보. 예를 들어, 레코드를 삭제하는 마이그레이션의 경우(대부분의 경우 자동으로 복구되지 않는 작업), 삭제된 레코드를 어떻게 복구할 수 있는지.
  • 마이그레이션이 데이터를 삭제하는 경우, ~data-deletion 레이블 적용.
  • 에러가 발생했을 때 사용자 경험에 가능한 영향을 간결하게 설명. 예를 들어, “이슈가 의도치 않게 에픽에서 사라질 수 있음”.
  • 쿼리 계획에서의 관련 데이터 포함. 예를 들어 수정 또는 삭제되는 레코드의 대략적인 수 등의 예상 데이터.

쿼리 추가 또는 수정 시 준비 사항

Raw SQL
  • MR(Merge Request) 설명에 원시 SQL 작성. 적절한 포맷팅을 선호하며, pgFormatter 또는 https://paste.depesz.com을 사용하고 일반적인 따옴표(예: "projects"."id")를 사용하고 스마트 따옴표(예: “projects”.“id”)를 피합니다.

  • 매개변수를 사용하여 동적으로 생성된 쿼리의 경우, 각 변형마다 하나의 원시 SQL 쿼리가 있어야 합니다.

    예를 들어, 프로젝트를 필터링하는 옵션이 있는 이슈를 찾는 경우, 이슈 버전과 프로젝트를 결합한 쿼리의 버전이 모두 포함되어야 합니다.

    모든 가능한 생성된 쿼리를 철저히 추가할 필요는 없습니다. 각 테이블의 나머지 테이블에 적합한 필터를 유지하면서 선택적으로 조인이나 그룹화 절 없이 생성된 쿼리의 버전도 포함해야 합니다.

  • 쿼리가 항상 한계와 오프셋과 함께 사용되는 경우, 최대 허용 한계가 사용되고 0이 아닌 오프셋이 포함되어야 합니다.
쿼리 계획
  • MR(Merge Request)에 포함된 각 원시 SQL 쿼리에 대한 쿼리 계획과 쿼리 계획 뒤에 따르는 링크 제공.
  • postgres.ai 챗봇에서 EXPLAIN ANALYZE를 실행하는 explain 명령으로 생성된 계획에 대한 링크 제공.
    • Database Lab에서 정확한 그림을 얻을 수 없는 경우, 개발 환경에 시드되어 EXPLAIN ANALYZE의 출력을 제공합니다. explain.depesz.com 또는 explain.dalibo.com을 사용하여 계획 및 사용된 쿼리를 붙여넣으세요.
  • 쿼리 계획 제공 시, 충분한 데이터를 얻을 수 있도록 해야 합니다:
    • 충분한 데이터로 쿼리 계획을 만들기 위해 다음의 ID를 사용할 수 있습니다:
      • 그룹을 포함하는 쿼리의 경우 gitlab-org 네임스페이스(namespace_id = 9970).
      • 프로젝트를 포함하는 쿼리의 경우 gitlab-org/gitlab-foss( project_id = 13083) 또는 gitlab-org/gitlab (project_id = 278964) 프로젝트.
        • 프로젝트 멤버십과 관련된 쿼리의 경우, 이러한 프로젝트의 project_namespace_id가 필요할 수 있습니다. 이것들은 15846663(for gitlab-org/gitlab) 및 15846626 (for gitlab-org/gitlab-foss)입니다.
      • 사용자를 포함하는 쿼리의 경우 gitlab-qa 사용자(user_id = 1614863).
        • 선택적으로 본인의 user_id를 사용하거나 프로젝트나 그룹 내에서 오랜 기간 활동한 사용자의 user_id를 사용할 수 있습니다.
    • 즉, 어떤 쿼리 계획도 0 또는 제공된 한계보다 적은 레코드를 반환해서는 안 됩니다 (한계가 포함된 경우). 쿼리가 배치에 사용될 경우, 적절한 예제 배치를 식별하고 제공해야 합니다.
    • 쿼리가 GitLab.com에서 데이터를 반환하지 않는 새로운 기능에 속하는 경우:
      • 쿼리를 분석하고 로컬 환경에서 계획을 제공할 수 있습니다.
      • postgres.ai를 사용하여 데이터를 업데이트(exec UPDATE issues SET ...)하고 새 테이블과 열을 생성할 수 있습니다(exec ALTER TABLE issues ADD COLUMN ...).
    • EXPLAIN 계획 이해하기에서 실제 반환된 레코드 수를 찾는 방법에 대한 자세한 정보 제공.
  • 쿼리 변경의 경우 변경 전과 후의 SQL 쿼리와 성능 개선을 나타내는 데이터를 모두 제공하는 것이 좋습니다.
  • 쿼리 계획을 평가할 때는 최종 쿼리가 데이터베이스에 실행되는 것이 필요합니다. finders 및 scopes로부터 반환된 중간 쿼리를 분석할 필요는 없습니다. PostgreSQL 쿼리 계획은 마지막 실행 전에 추가된 모든 최종 매개변수에 의존합니다. 실제 실행된 쿼리를 확인하는 한 가지 방법은 log/development.log을 확인하는 것입니다.

기존 테이블에 외래 키 추가 시 준비 사항

  • 외래 키를 추가하기 전에 소스 테이블에서 고아 행을 제거하는 마이그레이션 포함.
  • 더 이상 필요하지 않을 수도 있는 dependent: ...의 인스턴스 제거.

테이블 추가 시 준비 사항

  • 테이블 열 정렬 가이드라인을 따라 열을 정렬.
  • 다른 테이블에서 데이터를 가리키는 모든 열에 대해 외래 키 추가. 이에 따라 인덱스도 추가.
  • WHERE, ORDER BY, GROUP BY, JOIN과 같은 문에서 사용되는 필드에 대해 인덱스 추가.
  • 새 테이블은 반드시 db/fixtures/development/의 파일에 의해 씨딩되어야 합니다. 이러한 fixtures는 업그레이드가 성공적으로 완료되었는지 확인하고, 따라서 새 테이블은 항상 채워지도록 해야 합니다.
  • 새 테이블과 열은 필연적으로 위험하다는 것은 아니지만, 시간이 흐르면 일부 액세스 패턴은 본질적으로 확장하기 어려울 수 있습니다. 사전에 이러한 위험한 패턴을 식별하기 위해 액세스 및 크기에 대한 기대치를 문서화해야 합니다. MR(Merge Request) 설명에 다음 질문에 대한 답변 포함:
    • 다음 3개월, 6개월, 1년 동안 신규 테이블의 예상 성장은 얼마인가요? 이러한 가정은 무엇을 기반으로 하나요?
    • 다음 3개월, 6개월, 1년 동안 이 테이블이 시간당 얼마나 읽기 및 쓰기되는지는 어떻게 예상하나요? 어떤 상황에서 행이 업데이트되나요? 이러한 가정은 무엇을 기반으로 하나요?
    • 예상 데이터 볼륨 및 액세스 패턴을 고려하여 신규 테이블이 GitLab.com 또는 온프레미스 인스턴스에 가용성 리스크를 제공하나요? 제안된 디자인이 GitLab.com 및 온프레미스 고객의 요구를 지원할 수 있는 규모에 맞게 보장되나요?

컬럼, 테이블, 인덱스 또는 기타 구조물 제거 시 준비 사항

  • 컬럼 제거 가이드라인을 따르세요.
  • 일반적으로 삽입 후 마이그레이션에서 인덱스 및 외래 키를 제거하는 것이 더 좋지만(강제 규칙은 아님),
    • 예외로 작은 테이블용 인덱스 및 외래 키를 제거하는 경우가 있습니다.
  • 복합 인덱스를 추가하는 경우 다른 인덱스가 중복될 수 있으므로 동일한 마이그레이션에서 해당 인덱스를 제거하세요. 예를 들어 index(column_A, column_B, column_C)를 추가하면 index(column_A, column_B) 및 index(column_A)가 중복됩니다.

대량 업데이트 작업 사용 시 준비 사항

update, upsert, delete, update_all, upsert_all, delete_all 또는 destroy_all ActiveRecord 메서드를 사용하는 경우 데이터 수정 및 성능 저하를 주의해야 합니다. 또한 이러한 메서드는 공통 테이블 표현식(CTE)문과 호환되지 않을 수 있습니다. Danger는 이러한 메서드 사용 시 병합 요청 차이에 주석을 달겠습니다.

쿼리 추가 또는 수정 준비 사항을 위한 문서를 따라 원시 SQL 쿼리 및 쿼리 계획을 병합 요청 설명에 추가하고 데이터베이스 검토를 요청하세요.

데이터베이스 검토 방법

  • 마이그레이션 확인
    • 관계형 모델링 및 설계 선택 사항 검토
    • 데이터베이스 마이그레이션 스타일 가이드를 따르는 마이그레이션 검토
      • 예를 들어
      • 컬럼 정렬 확인
      • 외래 키에 대한 존재하는 인덱스 확인(migration_style_guide.md#adding-foreign-key-constraints)
    • 마이그레이션이 트랜잭션으로 실행되거나 동시 인덱스/외래 키 도우미만 포함되도록 보장
    • 대형 테이블에 인덱스가 추가되어 실행 시간이 늘어난 경우(1시간 이상) Database Lab에서
      • 이를 삽입 후 마이그레이션에서 추가되었는지 확인하세요.
      • 유지자: 병합 요청이 병합된 후 #f_upcoming_release Slack 채널에서 릴리스 매니저에게 알립니다.
    • db/structure.sql과 일치하는지, 그리고 마이그레이션이 가역적인지 확인
    • 관련 버전 파일이 db/schema_migrations에 추가 또는 제거되었는지 확인
    • 쿼리 시간 확인(있는 경우): 마이그레이션에서 실행된 누적 쿼리 시간이 15초에 편안하게 들어가야 합니다. 이보다 훨씬 짧은 것이 좋습니다. - GitLab.com의 경우
    • 컬럼 삭제 시, 해당 컬럼이 이전 릴리스에서 무시되었는지 확인
  • 일괄 배경 마이그레이션 확인:
    • GitLab.com에서 실행 시간 추정치 설정
      • 이력적인 목적으로, 병합 요청 설명에 해당 추정치를 포함하는 것이 매우 권장됩니다.
      • 예상 배치 수와 지연 간격의 곱으로 계산할 수 있습니다.
    • test 단계에서 수동으로 데이터베이스 테스트 작업(db:gitlabcom-database-testing)을 트리거
    • 단일 update가 1초 미만인 경우 쿼리를 정규 마이그레이션(db/migrate 내부)에 직접 넣을 수 있습니다.
    • 백그라운드 마이그레이션은 일반적으로 사용되지만 다음과 같은 경우에만 국한되지 않음:
      • 대형 테이블에서 데이터 마이그레이션
      • 데이터 집합의 레코드당 다수의 SQL 쿼리 수행
    • 쿼리 검토(예: 배치 크기가 적절한지 확인)
    • 보통 일반 마이그레이션보다 실행 시간이 오래 걸릴 수 있기 때문에, 백그라운드 마이그레이션을 포스트 마이그레이션으로 취급하는 것이 좋습니다: 이를 db/migrate 대신 db/post_migrate에 위치시키세요.
  • 마이그레이션에 대한 시간 상 가이드라인 확인
  • 마이그레이션이 가역적인지 확인하고 #down 메서드를 구현했는지 확인
  • 새 테이블 마이그레이션 확인:
    • 명시된 접근 패턴과 볼륨이 합리적인지 확인하세요. 기반이 되는 가정이 탄탄해 보이는지 확인하세요. 이러한 패턴이 안정성에 위험을 제공하는지 확인하세요.
    • 컬럼이 공간을 절약하기 위해 정렬되어 있는지 확인하세요.
    • 다른 테이블을 참조하는 외래 키가 있나요?
    • db/fixtures/development/에 테이블이 있는지 확인하세요.
  • 데이터 마이그레이션 확인:
    • GitLab.com 실행 시간 추정치 설정
    • 시간에 따라 데이터 마이그레이션은 정규, 삽입 후 또는 배경 마이그레이션에 배치될 수 있습니다.
    • 가능한 경우 데이터 마이그레이션도 가역적이어야 하거나 되돌릴 수 있는 방법에 대한 설명이 제공되어야 합니다. 모든 유형의 마이그레이션에 적용됩니다(정규, 삽입 후, 백그라운드).
  • 쿼리 성능
    • 지나치게 복잡한 쿼리 및 작성자가 특별히 검토하도록 한 쿼리를 확인하세요(있는 경우)
    • 없는 경우, 작성자에게 Database Lab을 사용하여 SQL 쿼리 및 쿼리 계획을 제공하도록 요청하세요.
    • 주어진 쿼리에 대해 데이터 분포와 관련된 매개변수를 검토하세요
    • 쿼리 계획을 확인하고 쿼리에 대한 개선 사항을 제안하세요(쿼리, 스키마 변경 또는 인덱스 추가 등)
    • 일반적 가이드라인은 쿼리가 100ms 실행 시간 아래에 들어오는 것
    • N+1 문제를 피하고 쿼리 횟수를 최소화하세요.

유용한 팁

  • 특정 브랜치에서 마이그레이션을 반복적으로 적용하고 되돌릴 경우 scripts/database/migrate.rb을 사용하여 이를 더 효율적으로 처리할 수 있습니다.