Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change OpLogFilter::agent_id to be optional #724

Open
sophie-cluml opened this issue Apr 23, 2024 · 24 comments · May be fixed by #826
Open

Change OpLogFilter::agent_id to be optional #724

sophie-cluml opened this issue Apr 23, 2024 · 24 comments · May be fixed by #826
Assignees

Comments

@sophie-cluml
Copy link
Contributor

sophie-cluml commented Apr 23, 2024

Background

  • 하나의 모듈에서 만들어진 로그뿐만 아니라 모든 모듈에서 만들어진 로그에 대한 검색을 허용하기 위해서, OpLogFilter::agent_idNone인 경우를 지원해야 합니다.

Tasks

@henry0715-dev henry0715-dev self-assigned this Sep 4, 2024
@henry0715-dev henry0715-dev linked a pull request Sep 11, 2024 that will close this issue
@henry0715-dev
Copy link
Contributor

henry0715-dev commented Sep 11, 2024

@sophie-cluml, @sehkone
OpLog Rocksdb key 변경에 대해서 문의하고자 합니다.

OpLogFilter agent_id Optional로 변경에 대한 요구사항을 만족하기 위해 내용을 분석해보았습니다.

내용파악

  • 기존 Rocksdb Key 구조(agent_id0timestamp)에서 agent_id에 대한 필터링 조건이 없는 경우 db full scan의 형태로 데이터를 가져와야 함.
  • range scan으로 가져오려면 agent_id에 대한 list를 미리 담아두고 있고, loop를 돌며 범위에 있는 agent_id에 대한 모든 데이터를 가져온 뒤 추가 처리 로직 필요함.
  • 코드에 대한 복잡도 및 성능이 매우 나쁠것으로 예상되며, 버그 발생시 해결이 어려움.

적용하기 위한 선적용 조건

  • Rocksdb key 구조 변경필요
  • OpLog의 Rockdb key구조를 timestamp0agent_id로 변경

key구조 변경시 이점

  • timestamp 구간으로 선 필터링 되기 때문에 agent_id값이 Optional이어도 full scan대비 성능이 좋을 것으로 예상
  • 코드 복잡도가 낮고, 재사용성도 높아서 다른 데이터들도 적용하기 쉬움

데이터 필터링 예시

적재된 데이터
1970-01-01 00:00:00.000000001 UTC 0 piglet@src 1
1970-01-01 00:00:00.000000002 UTC 0 piglet@src 1
1970-01-01 00:00:00.000000002 UTC 0 review@src 1
1970-01-01 00:00:00.000000003 UTC 0 piglet@src 1
1970-01-01 00:00:00.000000003 UTC 0 review@src 1
1970-01-01 00:00:00.000000004 UTC 0 piglet@src 1
1970-01-01 00:00:00.000000005 UTC 0 piglet@src 1

timestamp 3 - 5 구간 조회
1970-01-01 00:00:00.000000003 UTC 0 piglet@src 1
1970-01-01 00:00:00.000000003 UTC 0 review@src 1
1970-01-01 00:00:00.000000004 UTC 0 piglet@src 1

timestamp 3 - 5 구간, piglet 조회
1970-01-01 00:00:00.000000003 UTC 0 piglet@src 1
1970-01-01 00:00:00.000000004 UTC 0 piglet@src 1

test 소스코드 예시
https://github.com/aicers/giganto/pull/826/files#diff-446b968974ec54c01b9c4643bfccd39bbb699fbd8f13a7e5539e5ab95ab9998bR518-R550

@sophie-cluml
Copy link
Contributor Author

sophie-cluml commented Sep 12, 2024

OpLog가 저장되는 테이블의 키의 start key를 timestamp로 하고, mid key를 0을 하고, end key 를 agent id로 하자는 것으로 이해가 됩니다. 이에 대해 저는 동의하는 방향입니다. 이 테이블의 변경이 sensor 모듈이나 ai 엔진 모듈에의 영향도 없다고 보고 있습니다. UI 상에도 영향이 없을 것으로 보입니다. 마이그레이션 코드가 필요하다는 것을 제외하고는, 검색 상 UI에 모든/특정 agent, 시간 순서대로 보여줄 수 있는 작업도 간단해질 것으로 보입니다.

다만, 제안주신 사항들에 조금 더 정확히 sync가 되기 위하여, 다음 사항들에 대해 알려주시면 좋을 것 같습니다.

  • timestamp는 i64가 아닌 stringfied 된 값을 사용하는 것을 제안하시는 것인가요? 데이터 예시가 stringfied 된 값이라서 이것까지 제안하는 것인지에 대한 의문이 있습니다.
  • mid key 0을 넣는 것은 OpLog에서 꼭 필요한가요?
  • 데이터 예시 상, mid key 앞뒤로 space가 있는데, 혹시 mid key를 0 으로 제안하시는 것인가요?

@henry0715-dev
Copy link
Contributor

@sophie-cluml
답변드립니다.

  • 예시에서는 log를찍어 날짜를 보기 편하게 한것입니다.
  • mid key는 추가로 값을 넣은것은 아닙니다. 0은 기존에 값들 사이에 넣어주는 경계 데이터로 보입니다.
  • 현재 Oplog는 start key 0 timestamp 구조로 되어있습니다. 이를 timestamp 0 start key 형태로 순서만 바꾼것입니다.

@sophie-cluml
Copy link
Contributor Author

답변 감사합니다. 전체적인 방향에 대해 이해했습니다. 다만, 다음은 sync를 맞출 수 있으면 좋을 것 같습니다.

mid key는 추가로 값을 넣은것은 아닙니다. 0은 기존에 값들 사이에 넣어주는 경계 데이터로 보입니다.

이 부분은 KeyExtractor 와 test code 상 구현이 서로 상이한 점이 있어서, 어디를 보느냐에 따라 mid key를 무엇으로 보는지 차이가 발생한 것 같습니다. test code가 기존에 잘못 mid key를 0 으로 잡고 있는 것을 덕분에 발견하게 되어서, 이 이슈의 진행과 함께 또는 별개로, test code를 적절히 수정하는 것이 필요할 것 같습니다.

  • 프로덕션 코드

https://github.com/aicers/giganto/blob/main/src/graphql/log.rs#L87-L90
https://github.com/aicers/giganto/blob/main/src/ingest.rs#L903-L912

  • 테스트 코드

https://github.com/aicers/giganto/blob/main/src/graphql/log/tests.rs#L531-L536

@henry0715-dev
Copy link
Contributor

답변 감사합니다. 전체적인 방향에 대해 이해했습니다. 다만, 다음은 sync를 맞출 수 있으면 좋을 것 같습니다.

mid key는 추가로 값을 넣은것은 아닙니다. 0은 기존에 값들 사이에 넣어주는 경계 데이터로 보입니다.

이 부분은 KeyExtractor 와 test code 상 구현이 서로 상이한 점이 있어서, 어디를 보느냐에 따라 mid key를 무엇으로 보는지 차이가 발생한 것 같습니다. test code가 기존에 잘못 mid key를 0 으로 잡고 있는 것을 덕분에 발견하게 되어서, 이 이슈의 진행과 함께 또는 별개로, test code를 적절히 수정하는 것이 필요할 것 같습니다.

  • 프로덕션 코드

https://github.com/aicers/giganto/blob/main/src/graphql/log.rs#L87-L90 https://github.com/aicers/giganto/blob/main/src/ingest.rs#L903-L912

  • 테스트 코드

https://github.com/aicers/giganto/blob/main/src/graphql/log/tests.rs#L531-L536

0이 중간에 삽입된 것은 StorageKeyBuilder.start_key에서 넣어주고 있는 값입니다.
https://github.com/aicers/giganto/blob/main/src/storage.rs#L690
mid_key는 기존과 동일하게 None을 유지할 것입니다.

참고 : draft pr 올린곳에서는 아직 ingest 부분이 수정되어있진 않습니다.

@sophie-cluml
Copy link
Contributor Author

@henry0715-dev 제가 잘못 알고 있었네요. 알려주셔서 감사합니다!

@sophie-cluml
Copy link
Contributor Author

제 생각엔 이대로 진행해도 될것 같습니다. @sehkone 다른 의견 있으실까요?

@sehkone
Copy link
Contributor

sehkone commented Sep 22, 2024

  • 그렇게 할 경우 복수의 이벤트가 같은 키 값을 갖게 될 가능성은 없나요? 우리 중앙 서버의 경우 키 값으로, 앞에 64 bits의 timestamp를, 뒤에 64 bits의 serial number를 조합한 i128 값을 사용합니다.
  • 로그(OpLog)의 양은 트래픽 관련 다른 데이터에 비하여 현저하게 적을 것이므로, 굳이 키 안에 agent ID를 넣을 필요가 있는지도 생각해 봐야 할 것 같습니다.
  • 현재 구현되어 있는 키 생성 구조를 반드시 따를 필요는 없습니다.

@henry0715-dev
Copy link
Contributor

timestamp만 사용할 경우 동일한 시간대에 oplog가 들어온 경우 마지막 요청 agentId에 대해서만 값이 저장되어 기대한 결과를 얻지 못할 가능성이 있습니다.
따라서 timestamp + agentId 조합이 필요할 것으로 보입니다.

예시

 insert_oplog_raw_event(&store, "giganto", 5);
 insert_oplog_raw_event(&store, "review", 5);
 insert_oplog_raw_event(&store, "aice", 5);

조회 결과
1970-01-01 00:00:00.000000005 UTC, item : Some("aice@src 1")

@sehkone
Copy link
Contributor

sehkone commented Sep 23, 2024

제 질문은 같은 agent id로부터도 동일한 timestamp 입력이 없냐는 것이었어요.

@henry0715-dev
Copy link
Contributor

reproduce에서 전송하는 timestamp 구조상으로는 각 모듈에서 저장하고 있는 log의 timestamp 값을 보내주기 때문에
해당 모듈에서 로깅시에 문제가 발생하지 않는한 동일한 timestamp 입력의 가능성은 없을것 같습니다.

로깅되는 예시
스크린샷 2024-09-23 오전 11 08 00

@sehkone
Copy link
Contributor

sehkone commented Sep 23, 2024

그러니까 제 질문은 각 모듈에서 timestamp가 동일한 것으로 찍힐 가능성이 아예 없냐는 것이에요. 우리 모듈들이 멀티 쓰레딩으로 동작하고 있기 때문에 질문하는 것입니다.

@henry0715-dev
Copy link
Contributor

멀티 쓰레딩으로 동작하는 환경에서 timestamp 동일한 것으로 찍힐 가능성은 있습니다.
이를 완전히 해결하기 위해서는 RDB처럼 ID를 발급해서 적용하거나, thread id 같은것을 추가로 로깅한뒤 그 값을 key에 포함시키는 방법이 있을것으로 생각 됩니다.

@sehkone
Copy link
Contributor

sehkone commented Sep 23, 2024

멀티 쓰레딩으로 동작하는 환경에서 timestamp 동일한 것으로 찍힐 가능성은 있습니다. 이를 완전히 해결하기 위해서는 RDB처럼 ID를 발급해서 적용하거나, thread id 같은것을 추가로 로깅한뒤 그 값을 key에 포함시키는 방법이 있을것으로 생각 됩니다.

"로깅"은 각 모듈이 하는 것이고 key를 만드는 것은 Giganto가 하는 것입니다. Thread ID를 추가하여 로깅한다는 말은 각 모듈이 그렇게 한다는 것인가요? 매 로그마다 thread ID를 넣자는 것인가요? 적절해 보이지 않는데요.

@sehkone
Copy link
Contributor

sehkone commented Sep 23, 2024

Timestamp를 키로 사용할 때 중복을 방지하는 방식으로 우리가 현재 사용 중인 방법이 두 가지가 있습니다.

  1. 위에서 적은 것처럼 128비트의 키를 만드는데 timestamp와 serial number를 결합
  2. Timestamp의 값 중에서 사용하지 않는 몇 자리의 비트를 활용

@henry0715-dev
Copy link
Contributor

기존에 구현되어있는 key 구조 agentId-0-timestamp 도 동일한 이슈가 있을 수 있어 보입니다.
key구조에 대한 아이디어로는 mid key값을 현재는 셋팅 None으로 하고 있는데, 이 부분에 데일리로 카운팅 될 수 있는 숫자를 넣는 것으로 활용할 수 있을 것 같습니다.

@sehkone
Copy link
Contributor

sehkone commented Sep 23, 2024

기존에 구현되어있는 key 구조 agentId-0-timestamp 도 동일한 이슈가 있을 수 있어 보입니다.
key구조에 대한 아이디어로는 mid key값을 현재는 셋팅 None으로 하고 있는데, 이 부분에 데일리로 카운팅 될 수 있는 숫자를 넣는 것으로 활용할 수 있을 것 같습니다.

이것과 관련해서는 제가 이미 아래와 같이 이야기를 했습니다. 즉, agentId-0-timestamp 이 구조는 더 이상 논의의 대상이 아닙니다. 저는 agent ID도 없애면 어떠냐, 그리고 심지어 셋으로 구분되어 있는 키 구조를 사용하지 않으면 어떠냐, 라고 이야기한 것이지요.

  • 로그(OpLog)의 양은 트래픽 관련 다른 데이터에 비하여 현저하게 적을 것이므로, 굳이 키 안에 agent ID를 넣을 필요가 있는지도 생각해 봐야 할 것 같습니다.
  • 현재 구현되어 있는 키 생성 구조를 반드시 따를 필요는 없습니다.

@henry0715-dev
Copy link
Contributor

key 구조에 대해서는 timestamp와 serial number를 결합에 대해서 잡아보도록 하겠습니다.
코멘트 감사드립니다.

@henry0715-dev
Copy link
Contributor

�다른 작업으로 인하여 pending 상태로 변경합니다.

@henry0715-dev
Copy link
Contributor

@sehkone
문의 사항이 있습니다.
Oplog 저장시에 agent_name 영역에 agent_id만 저장되고 있는 형태에서 -> agent_id@hostname 형태로 수정하여 저장하는 것을 고려 중입니다.
이유 :

  • Oplog를 조회한다고 가정하였을때, 기존에는 key값에 agent_id@hostname 값이 포함되어 들어가서 조회가 가능하였지만, 변경되는 key 형태(timestamp+sequnece)에서는 source(hostname)가 어디인지 확인이 어려울것이란 생각이 듭니다.
  • Cluster를 구성하여 Oplog를 쌓는 경우 1,2,3 서버 중 어디에서 log가 쌓였는지 화면에서 볼수 있어야 할것이란 생각이 듭니다.

수정 관련 코드 : git code

스크린샷 2024-10-23 오후 3 30 28

cc @sophie-cluml

@sehkone
Copy link
Contributor

sehkone commented Oct 25, 2024

@sehkone 문의 사항이 있습니다. Oplog 저장시에 agent_name 영역에 agent_id만 저장되고 있는 형태에서 -> agent_id@hostname 형태로 수정하여 저장하는 것을 고려 중입니다. 이유 :

  • Oplog를 조회한다고 가정하였을때, 기존에는 key값에 agent_id@hostname 값이 포함되어 들어가서 조회가 가능하였지만, 변경되는 key 형태(timestamp+sequnece)에서는 source(hostname)가 어디인지 확인이 어려울것이란 생각이 듭니다.

OpLog의 필드로 source가 없나요?

@henry0715-dev
Copy link
Contributor

@sehkone 문의 사항이 있습니다. Oplog 저장시에 agent_name 영역에 agent_id만 저장되고 있는 형태에서 -> agent_id@hostname 형태로 수정하여 저장하는 것을 고려 중입니다. 이유 :

  • Oplog를 조회한다고 가정하였을때, 기존에는 key값에 agent_id@hostname 값이 포함되어 들어가서 조회가 가능하였지만, 변경되는 key 형태(timestamp+sequnece)에서는 source(hostname)가 어디인지 확인이 어려울것이란 생각이 듭니다.

OpLog의 필드로 source가 없나요?

현재 아래와 같이 구성되어있습니다.

pub struct OpLog {
    pub agent_name: String,
    pub log_level: OpLogLevel,
    pub contents: String,
    // Category, id
}

@sehkone
Copy link
Contributor

sehkone commented Oct 25, 2024

그렇다면, 다른 타입과 마찬가지로 source 필드가 들어가는 게 더 낫습니다.

@henry0715-dev
Copy link
Contributor

REproduce 쪽 테스트 포함해서 source 필드 추가하는 것으로 검토해보도록 하겠습니다.

henry0715-dev added a commit that referenced this issue Nov 4, 2024
Close #724

The RocksDB keys in the `oplog` are currently in the form agent_id-0-timestamp.
This change modifies the keys to the form timestamp-0-sequence.
henry0715-dev added a commit that referenced this issue Nov 5, 2024
Close #724

The RocksDB keys in the `oplog` are currently in the form agent_id-0-timestamp.
This change modifies the keys to the form timestamp-0-sequence.
henry0715-dev added a commit that referenced this issue Nov 5, 2024
Close #724

The RocksDB keys in the `oplog` are currently in the form agent_id-0-timestamp.
This change modifies the keys to the form timestamp-0-sequence.
henry0715-dev added a commit that referenced this issue Nov 7, 2024
Close #724

The RocksDB keys in the `oplog` are currently in the form agent_id-0-timestamp.
This change modifies the keys to the form timestamp-0-sequence.
henry0715-dev added a commit that referenced this issue Nov 8, 2024
Close #724

The RocksDB keys in the `oplog` are currently in the form agent_id-0-timestamp.
This change modifies the keys to the form timestamp-0-sequence.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants