Skip to content

[feature] 픽 실시간 업데이트 #174

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

Merged
merged 11 commits into from
Apr 28, 2025
Merged

Conversation

meanjoo
Copy link
Collaborator

@meanjoo meanjoo commented Mar 3, 2025

#️⃣연관된 이슈

📝작업 내용 및 코드

결과
다른 사용자가 새로운 픽을 추가/삭제했을 때 이를 지도에서 보려면 지도를 움직여야 했던 것을 실시간으로 지도 화면에서 반영되도록 했습니다.

  • 기존

    • 왼쪽을 사용자 A, 오른쪽을 사용자 B라고 해봅시다.
      B는 A의 현재 위치를 포함한 지도 화면을 보고 있습니다. B는 지도에서 아무것도 하지 않고 있을 때 A가 현재 위치에 픽을 등록해도 B는 바로 업데이트 받을 수 없습니다. B는 지도 화면을 움직이거나 다른 화면으로 이동하고 돌아온 후에야 A가 등록한 픽을 확인할 수 있습니다.

    before

    • 지도를 움직이지 않고 다른 사용자도 등록된 픽을 확인할 수 있게 실시간 업데이트가 되면 좋을 것 같다고 생각했습니다.
  • 현재

    1.mp4

과정

addSnapshotListener에 대한 검토

참고) 공식 문서

실시간 업데이트를 위해 query.get()을 사용하던 것을 query.addSnapshotListener()를 사용해도 될 지에 대해 공식 문서를 보며 해당 리스너에 대해 알아보고 검토했다.

현재 프로젝트에서는 픽 데이터 하나를 Firestore에서 picks 컬렉션의 문서 하나로 하여 진행하고 있다. 따라서 픽 리스트 실시간 업데이트라 하면 컬렉션에서 문서의 추가 및 삭제가 감지될 수 있는지를 알아야 했다.

사실 처음에는 공식 문서의 첫 문장이 “You can listen to a document with the onSnapshot() method.” 라고 되어 있어 안 되는 줄 알았다. (ㅎㅎ;) 문서를 읽다 보니 아래에 컬렉션에서 문서를 listen 할 수 있는 방법에 대해 나와있어 가능하다는 것을 파악했다.

image

다음으로 알아본 건 최초의 데이터는 어떻게 되는 건지였다. 실시간 변경에 대한 기준 데이터(초기 상태)를 만들어줘야 하는지와 같이 따로 어떤 처리가 필요한지 알아야 했다. 역시나 문서에서도 Important로 나와있는 내용이었다.

image (1)

첫 번째 쿼리 스냅샷은 쿼리와 일치하는 모든 존재하는 문서에 대한 ADDED 이벤트를 포함한다고 되어 있다. 처음에는 쿼리와 일치하는 모든 문서를 추가로 보고 이걸 기준으로 변하는 데이터를 보는 것이다. 따라서 초기 상태를 따로 처리하는 과정은 필요 없다.

이렇게 검토를 마치고 DataSource에서 픽 데이터를 가져오는 부분 중 query.get()query.addSnapshotListener()로 바꾸는 작업을 시작했다.

fetchPicksInArea() 수정

기존의 fetchPicksInArea() 함수 반환 타입(List)을 유지하려고 한다면 데이터 변경이 있을 때마다 matchingPicks를 return 해야 할 것인데 이는 너무 비효율적인 방식이라 생각했다. 그리고 찾아보니 안드로이드 공식 문서 - 데이터 레이어 내용 중 시간에 따른 데이터 변경을 감지하려면 data layer에서 flow를 노출하라고 되어 있었다.

실시간 픽 업데이트를 위해 FirebaseRepository와 FirebaseRemoteDataSource의 반환 타입을 Flow로 변경했다.

  • FirebaseRepository : Result<List<Pick>>Flow<List<Pick>>
  • FirebaseRemoteDataSource : List<Pick>Flow<List<Pair<PickType, Pick>>>
// (domain) FirebaseRepository.kt
interface FirebaseRepository {
    suspend fun fetchPicksInArea(lat: Double, lng: Double, radiusInM: Double): Flow<List<Pick>>
}

// (domain) FirebaseRemoteDataSource.kt
enum class PickType {
    UPDATED, REMOVED
}

interface FirebaseRemoteDataSource {
    suspend fun fetchPicksInArea(lat: Double, lng: Double, radiusInM: Double): Flow<List<Pair<PickType, Pick>>>
}

여태 작성한 코드들을 보면 Repository와 DataSource에서 이름이 같은 함수의 반환 타입은 Result로 감싸진 차이만 있지 타입은 같았다. 하지만 이번에 픽 실시간 업데이트를 진행하며 Repository와 DataSource의 fetchPicksInArea() 함수가 Flow에서 방출하는 타입을 다르게 설정했다.

두 타입을 같게 하려면 Flow<List<Pick>>으로 같게 할 수는 있지만 다르게 한 이유는 이렇게 하면 DataSource가 너무 많은 책임을 맡게 되고 Repository가 무의미하다고 생각했기 때문이다.

FirebaseRepository와 FirebaseRemoteDataSource의 fetchPicksInArea()가 하는 일은 Firebase에서 데이터를 받고 UI에서 사용하기 위해 현재 화면에 있는 픽 데이터를 리스트로 만들어서 내보내주는 것이다. 만약 두 함수의 반환 타입을 같게 하려면 이 모든 과정을 FirebaseRemoteDataSource에서 진행해야 하고 FirebaseRepository는 그저 DataSource를 감싸는 것 밖에 안 된다.

만약 DataSource에 모든 게 있다면 최신 근처 픽 데이터도 여기에서 관리해야 하는데 DataSource에 데이터를 유지시키는 건 별로라고 생각했다.

따라서 data source는 firestore의 데이터를 올려보내는 용, repository는 data source에서 받은 데이터를 처리하는 쪽으로 하고 둘의 반환 타입을 다르게 했다. repository에서 픽 리스트를 관리하기 위해서는 받은 픽이 삭제된 건지 새로 생긴 건지를 알아야 하기 때문에 data source에서 보내는 데이터에는 픽뿐만 아니라 그 픽이 어떤 타입(UPDATED, REMOVED)인지도 포함되어야 한다. repository에서 보내는 데이터에는 최신 주변 픽 리스트면 되기 때문에 타입이 필요 없다.

FirebaseDataSourceImpl.kt

  • 기존 함수

    override suspend fun fetchPicksInArea(
        lat: Double,
        lng: Double,
        radiusInM: Double
    ): List<Pick> {
        val center = GeoLocation(lat, lng)
        val bounds = GeoFireUtils.getGeoHashQueryBounds(center, radiusInM)
    
        val queries: MutableList<Query> = ArrayList()
        val tasks: MutableList<Task<QuerySnapshot>> = ArrayList()
        val matchingPicks: MutableList<Pick> = ArrayList()
    
        bounds.forEach { bound ->
            val query = db.collection("picks")
                .orderBy("geoHash")
                .startAt(bound.startHash)
                .endAt(bound.endHash)
            queries.add(query)
        }
    
        try {
            queries.forEach { query ->
                tasks.add(query.get())
            }
            Tasks.whenAllComplete(tasks).await()
        } catch (exception: Exception) {
            Log.e("FirebaseDataSourceImpl", "Failed to fetch picks", exception)
            throw exception
        }
    
        tasks.forEach { task ->
            val snap = task.result
            snap.documents.forEach { doc ->
                if (isAccurate(doc, center, radiusInM)) {
                    doc.toObject<FirebasePick>()?.run {
                        matchingPicks.add(this.toPick().copy(id = doc.id))
                    }
                }
            }
        }
    
        return matchingPicks
    }
  • 변경 후 함수

    override suspend fun fetchPicksInArea(
        lat: Double,
        lng: Double,
        radiusInM: Double
    ): Flow<List<Pair<PickType, Pick>>> = callbackFlow {
        val listeners = mutableListOf<ListenerRegistration>()
        try {
            val center = GeoLocation(lat, lng)
            val bounds = GeoFireUtils.getGeoHashQueryBounds(center, radiusInM)
    
            bounds.forEach { bound ->
                val query = db.collection(COLLECTION_PICKS)
                    .orderBy("geoHash")
                    .startAt(bound.startHash)
                    .endAt(bound.endHash)
    
                val listener = query.addSnapshotListener { snapshots, e ->
                    if (e != null) {
                        Log.w("SnapshotListener", "listen:error", e)
                        close(e)
                        return@addSnapshotListener
                    }
    
                    val pickData = mutableListOf<Pair<PickType, Pick>>()
                    for (dc in snapshots!!.documentChanges) {
                        if (isAccurate(dc.document, center, radiusInM)) {
                            dc.document.toObject<FirebasePick>().run {
                                when (dc.type) {
                                    DocumentChange.Type.ADDED, DocumentChange.Type.MODIFIED -> {
                                        pickData.add(PickType.UPDATED to this.toPick().copy(id = dc.document.id))
                                    }
                                    DocumentChange.Type.REMOVED -> {
                                        pickData.add(PickType.REMOVED to this.toPick().copy(id = dc.document.id))
                                    }
                                }
                            }
                        }
                    }
                    trySend(pickData).isSuccess
                }
                listeners.add(listener)
            }
        } catch (e: Exception) {
            close(e)
        }
    
        // Flow 종료 시 모든 리스너 제거
        awaitClose { listeners.forEach { it.remove() } }
    }

기존의 함수와 구조는 크게 달라진 게 없다. 기존에는 query.get()를 사용했는데 query.addSnapshotListener()를 사용하여 데이터가 변경될 때마다 알 수 있도록 했다.

FirebaseRepositoryImpl.kt

  • 기존 함수

    override suspend fun fetchPicksInArea(
        lat: Double,
        lng: Double,
        radiusInM: Double
    ): Flow<List<Pick>> {
    		val pickList = firebaseRemoteDataSource.fetchPicksInArea(lat, lng, radiusInM)
    		return handleResult(FirebaseException.NoSuchPickInRadiusException()) {
    		    pickList.ifEmpty { null }
    }
  • 변경 후 함수

    private val latestNearPickMutex = Mutex()
    
    override suspend fun fetchPicksInArea(
        lat: Double,
        lng: Double,
        radiusInM: Double
    ): Flow<List<Pick>> {
    		private val latestNearPick = mutableMapOf<String, Pick>()
    		
        return firebaseRemoteDataSource.fetchPicksInArea(lat, lng, radiusInM)
            .map { pickList ->
                latestNearPickMutex.withLock {
                    pickList.forEach { (type, pick) ->
                        when (type) {
                            PickType.UPDATED -> {
                                latestNearPick[pick.id] = pick
                            }
    
                            PickType.REMOVED -> {
                                latestNearPick[pick.id]?.let {
                                    latestNearPick.remove(pick.id)
                                }
                            }
                        }
                    }
                    latestNearPick.values.toList()
                }
            }
    }

기존에는 data source에서 반환된 리스트가 비어있으면 NoSuchPickInRadiusException을 발생시키고 그렇지 않으면 data source에서 반환된 리스트를 반환했다.

변경 후에는 일단 data source에서 반환된 리스트의 요소 타입이 Pair<PickType, Pick>로 바뀌었다. 그리고 repository에서 최신 근처 픽 데이터를 관리하도록 했다. 최신 근처 픽 데이터를 관리하는 타입이 맵인 것이 리스트인 것보다 삭제할 때 효율적이라고 생각하여 맵으로 설정했다. data source에서 온 데이터에서 PickType에 따라 새로 생긴 픽(UPDATED)이라면 최신 근처 픽 데이터에 추가하고 삭제된 픽(REMOVED)이라면 최신 근처 픽 데이터에서 삭제했다. 마지막에는 최신 근처 픽 데이터 리스트를 반환한다. Mutex가 필요한 게 확실한지는 모르겠지만 그래도 혹시 몰라 Mutex를 사용해두어 동시에 여러 코루틴이 latestNearPick을 수정하는 것을 막아두었다.

마주했던 문제 1

오른쪽 사용자가 어떤 클러스터 마커의 리스트를 보고 있을 때 왼쪽 사용자가 오른쪽 사용자가 보고 있는 클러스터 마커에 포함되는 픽을 추가했을 때, 오른쪽 사용자의 클러스터 마커에 표시된 숫자는 변하지만 리스트에는 추가한 픽이 보이지 않는 문제가 있었다.

문제1 (1)

기존에는 마커 상태 업데이트를 뷰모델의 함수에서 했었는데 이를 Clusterer 쪽으로 옮겼더니 해결되었다.

  • 기존
    기존에는 fetchPickUseCase()로 픽 리스트를 가져온 후 마커 상태 업데이트를 뷰모델의 함수 안에서 했다.
// MapViewModel.kt
fun fetchPicksInBounds(leftTop: LatLng, clusterer: Clusterer<MarkerKey>?) {
    viewModelScope.launch {
        _centerLatLng.value?.run {
            val radiusInM = leftTop.distanceTo(this)
            fetchPickUseCase(this.latitude, this.longitude, radiusInM)
                .onSuccess { pickList ->
                    val newKeyTagMap: MutableMap<MarkerKey, String> = mutableMapOf()
                    pickList.forEach { pick ->
                        newKeyTagMap[MarkerKey(pick)] = pick.id
                        _picks[pick.id] = pick
                    }
                    _clickedMarkerState.value.clusterPickList?.let { clusterPickList -> // 클러스터 마커가 선택되어 있는 경우
                        val updatedPickList = mutableListOf<Pick>()
                        clusterPickList.forEach { pick ->
                            _picks[pick.id]?.let { updatedPick ->
                                updatedPickList.add(updatedPick)
                            }
                        }
                        _clickedMarkerState.emit(_clickedMarkerState.value.copy(clusterPickList = updatedPickList.toList())) // 최신 픽 정보로 clusterPickList 업데이트
                    }
                    clusterer?.addAll(newKeyTagMap)
                }
                .onFailure {
                    // TODO: NoSuchPickInRadiusException일 때
                    Log.e("MapViewModel", "${it.message}")
                }
        }
    }
}
  • 현재
    뷰모델에서 마커 상태를 관리하는 값 중 prevClickedMarker가 있는데 이는 이전에 클릭된 마커로 현재 클릭되어 있는 마커이다. 클러스터러 내부에서 만약 마커의 위치가 현재 클릭되어 있는 마커와 같다면 마커 상태를 업데이트하도록 했다.
// Clusterer.kt
.clusterMarkerUpdater(object : DefaultClusterMarkerUpdater() {
		override fun updateClusterMarker(info: ClusterMarkerInfo, marker: Marker) {
				// ...
				if (mapViewModel.clickedMarkerState.value.prevClickedMarker?.position == marker.position) {
            mapViewModel.clickedMarkerState.value.clusterPickList?.let {
                mapViewModel.setClickedMarkerState(
                    context = context,
                    marker = marker,
                    clusterTag = info.tag.toString()
                )
            }
        }
        
        // ...
    }
}
.leafMarkerUpdater(object : DefaultLeafMarkerUpdater() {
		override fun updateLeafMarker(info: LeafMarkerInfo, marker: Marker) {
				// ...
				if (mapViewModel.clickedMarkerState.value.prevClickedMarker?.position == marker.position) {
            mapViewModel.setClickedMarkerState(
                context = context,
                marker = marker,
                pickId = pick.id
            )
        }
        
        // ...
		}
}

문제 1 해결

💬리뷰 요구사항 및 참고 사항

  • 설정을 뭔가 잘못 건드렸는지 세 번째 커밋 메시지가 깨졌네요 ;.. 작성했던 커밋 메시지는 내 주변 반경 픽 리스트 업데이트입니다.

  • 아직 주석이나 로그가 일부 정리되지 않았는데 참고 부탁드립니다.

  • Repository와 DataSource의 반환 타입이나 역할에 대해 제가 위에 써둔 게 있는데 다른 분들은 어떻게 생각하시는지 궁금합니다.

  • Flow로 변경하며 NoSuchPickInRadiusException을 사용하지 않게 되었는데 삭제해도 될까요?

  • 단말 마커를 누르고 있는 상태에서 그 위치에 픽이 추가되어 클러스터 마커가 되면 클릭이 해제됩니다. 이에 대해 어떻게 생각하시나용? 사실 이 경우에 마커는 클러스터 마커가 되고 인포윈도우가 떠있게 하고 싶었는데 어떻게 해야할 지 잘 모르겠습니다.
    문제1) 단말 마커를 누르고 있을 때 추가 되면 선택 해제

  • 이전에 마커를 눌러보고 화면을 이동하고 이것저것 했을 때 아주 가끔 마커가 클릭되지 않는 문제가 있었는데 어느 상황에서 이런 문제가 발생했었는지 정확히 기억이 나지 않습니다. 고쳤다고 생각하는데 혹시 여전히 이런 현상이 있다면 언제 발생하는지 알려주세요!

  • 3월 3일 월요일 회의에서 제가 공유했던 문제 중 일부(삭제 관련)를 수정했는데 여전히 다른 문제들이 있을 수 있습니다. 혹시 발견하신다면 알려주시면 감사하겠습니다!

기타

  • 이건 제가 픽을 등록하며 본 건데 a를 검색했을 때 가장 위에 검색되는 게 달라달라이고 그 아래 것들도 뭔가 a를 검색했을 때 이게 나오는 게 맞는 건가? 싶은 것들이 있습니다. 뭔가 확인해보면 좋을 것 같아 여기에 작성합니다.

    a 검색 시 애플 뮤직에서 a 검색 시

@meanjoo meanjoo added the feature 기능 개발 label Mar 3, 2025
@meanjoo meanjoo requested review from yuni-ju and miller198 March 3, 2025 19:07
@meanjoo meanjoo self-assigned this Mar 3, 2025
@miller198
Copy link
Collaborator

image

Setting - Editor - Code Style - Kotlin 들어가면 위 사진처럼 Hard wrap at 이라는 항목이 있는데
자동 줄바꿈 리미트 설정할 수 있어요
전 보통 한번에 볼 수 있을 정도로 설정해놓는데 120~130자 정도가 적당해보이더라구요
혹시 줄바꿈이 너무 잦게 일어나면 한번 설정해보세요

@meanjoo
Copy link
Collaborator Author

meanjoo commented Mar 4, 2025

@miller198 style 커밋 때문에 알려주신 것 같은데 알려주셔서 감사합니다!
여태까지 자동 정렬에 크게 신경 쓰이는 점이 없어서 딱히 설정을 건드리지 않아서 확인해보니 저는 디폴트인 100으로 되어 있네요

이 값이 다르면 서로 코드 자동 정렬하는 게 다를 것 같아 이 부분도 프로젝트 내에서 통일하면 좋을 것 같네요
저는 100이든 130이든 다 괜찮아서 승규님이 올려주신 사진에 있는 값인 130으로 맞출까요? 윤겸님 의견도 들어보고 하나의 값으로 정하면 좋을 것 같습니다
만약 100이 아닌 다른 값으로 맞추게 된다면 다시 코드 정렬하고 커밋 올리겠습니당

@yuni-ju
Copy link
Collaborator

yuni-ju commented Mar 4, 2025

저는 반대로 200으로 쓰고있었네요! ㅋㅋ 130으로 맞추는 것 괜찮은 것 같습니당

Copy link
Collaborator

@yuni-ju yuni-ju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

실시간으로 화면에 픽이 생성되고 삭제되는 기능 잘 동작함을 확인했습니다 😄 (테스트도 재밌네요 ㅎㅎ)

  1. Repository와 DataSource에서 타입 바꾸신 것 말씀하신대로 역할이 분명히 분리되어 있다면 괜찮다고 생각합니다. 그리고 사용하지 않는 Exception 삭제해도 될 것 같아요!

  2. 마커 클릭 해제에 대하여 만약 개선하고 싶다면 클러스터 상태가 변경될 때 콜백을 실행하는 방식으로 구현할 수 있을 것 같습니다.

하지만 만약 상태 변경이 계속 일어나서 사용자가 특별한 조작을 하지 않았음에도 단말클릭상태 <-> 클러스터 선택 상태가 반복해서 변경된다면 이 또한 사용자에게 더 큰 혼동을 줄 수 있을 것이라 생각합니다. (클릭하려고 하는데 자꾸 바뀌는 등의 혼란이 있을 수 있을 것 같아요 @_@)

그래서 개인적인 의견으로는 이 부분은 오히려 클릭한 상태일 때는 사용자가 추가적인 동작을 했을 때 변경되도록 두는 것도 괜찮다고 생각합니다.

  1. Mutex 처리해주신 것 좋습니다!

Comment on lines 186 to 190
val nearPickSet = mutableSetOf<Pick>()
fetchPickUseCase(location.latitude, location.longitude, notiRadius)
.onSuccess {
_nearPicks.emit(it)
}.onFailure {
_nearPicks.emit(emptyList())
.collect { pickList ->
nearPickSet.addAll(pickList)
_nearPicks.emit(nearPickSet.toList())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기는 pick의 중복방지를 위해 set을 사용하신 것 같은데, 중복으로 불러와지는 경우가 어떨 때 발생하나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그러게요 ㅎㅎ..? 저기에서 set을 사용할 이유가 없어보이네요 삭제했습니다!

제가 처음에는 repository에서 해당 영역에 존재하는 리스트를 반환하는 게 아니라 추가되는 픽들만 반환하는 식으로 구상했어서 그랬던 것 같네용

queries.add(query)
}

): Flow<List<Pair<PickType, Pick>>> = callbackFlow {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pair나 Triple 같은 타입은 구현하는 사람 입장에선 이해가 쉽지만 다른 개발자 입장에선 이해가 힘들 수 있어 사용을 자제하라는 말을 들었어요
찾아보니 이 글에서 비슷한 내용이 언급되어있네요

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 Pair는 .first, .second로 값에 접근하기 때문에 유지보수 측면에서 어려울 수 있어 사용을 자제하라는 말은 많이 들어서 알고 있었습니다. 그래서 평소에 Triple은 전혀 사용하지 않고 Pair도 잘 사용하지 않는데 그럼에도 Pair를 사용했던 이유는 코드에서 .first, .second로 값에 접근하지 않았고, 타입이 PickType, Pick이라는 것에서 이미 무슨 값인지 잘 알 수 있겠지? 라는 생각 때문이었습니다.

구조 분해를 사용하여 first, second가 아닌 type, pick으로 값에 접근했었습니다.

firebaseRemoteDataSource.fetchPicksInArea(lat, lng, radiusInM)
    .map { pickList ->
            pickList.forEach { (type, pick) ->
    }
}

그런데 생각해보니 이건 코드를 작성한 제 입장이고 다른 사람들이 읽었을 땐 아닐 수 있다는 걸 다시 한 번 머리에 넣어두겠습니다. 언급해주신 글을 보고 Pair나 Triple이 lazy data class라는 건 몰랐는데 알아갑니다!

Pair를 사용하던 것에서 PickWithType이라는 data class를 만들어서 수정했는데 어떤가요?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋습니다!!

}
trySend(pickData).isSuccess
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isSuccess가 필요한가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음 아니요.. 필요없을 것 같습니다

}
} catch (e: Exception) {
close(e)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기나 위의 close(e)가 호출되면 이 callbackFlow를 받는 repository 코드에서 catch 로 예외 e에 대한 처리를 해야하지않을까요?

@@ -13,7 +19,14 @@ class FirebaseRepositoryImpl @Inject constructor(
private val firebaseRemoteDataSource: FirebaseRemoteDataSource
) : FirebaseRepository {

override suspend fun createGoogleIdUser(uid: String, email: String, userName: String?, userProfileImage: String?): Result<User> {
private val latestNearPickMutex = Mutex()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 이 뮤텍스가 왜 필요한지 자세히 설명해주실수있나요
동시성 문제가 언제 발생하는지 잘 이해가 안되서요 ㅠ

Copy link
Collaborator Author

@meanjoo meanjoo Mar 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

으음 제가 PR을 올리며 작성한 글에도 Mutex가 필요한 게 확실한 건지 모르겠다는 말을 쓰긴 했지만 ㅎ.. 제가 동시성 문제가 발생할 거라고 생각한 경우는 아래와 같습니다

픽이 어떤 기준으로 불러와지는지는 알고 계실 거라 생각해요 참고
현재 구현된 코드에서는 Geohash 필드마다 flow가 하나씩 존재합니다.

image

예를 들어서 현재 보고 있는 화면의 지도 영역이 위와 같다고 하면 Geohash가 ezz, u00 외에도 몇 개가 있을 겁니다. 사용자 A와 B가 각각 ezz, u00에서 픽을 하나씩 동시에 추가한다고 하면 ezz flow에서 값(새로 추가된 픽)을 방출하고, u00 flow에서도 값을 방출합니다. 만약 뮤텍스를 사용하지 않으면 두 flow에서 기존 리스트 + 새로 추가된 픽을 따로 업데이트하여 latestNearPick이 제대로 된 최신 주변 픽 리스트가 아닐 거라 생각했습니다. 기존 리스트가 30개라고 해봅시다. 보고 있는 화면에서 2개가 추가되면 리스트가 32개여야 할 겁니다. 뮤텍스가 없으면 두 flow에서 기존의 리스트에 동시에 접근할 수도 있다고 생각합니다. 그렇게 되면 아래처럼 될 거라 생각했습니다

  • ezz flow : 기존 리스트(30개)에 ezz 영역에 새로 추가된 픽이 추가된 리스트 -> 31개
  • u00 flow : 기존 리스트(30개)에 u00 영역에 새로 추가된 픽이 추가된 리스트 -> 31개

뮤텍스를 사용한다면 여러 개의 flow가 동시에 latestNearPick에 접근하지 않게 되어 30개에서 32개로 안전하게 업데이트될 것이라고 생각했습니다.

제가 필요한 게 확실한 건지 모르겠다고 한 이유는 이런 상황을 직접 보고 확인해보지는 못해서ㅜ입니다.. 혹시 제 생각이 틀린 것 같다면 알려주세요!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗 리뷰한지가 오래되었다가 다시 코드를 보니 mutex가 필요없는 것 같다고 생각합니다..!

현재 FirebaseRepositoryImpl파일의 fetchPicksInArea에서 사용되는 latestNearPick이 지역 변수로 선언되어 있습니다.
여러 개의 fetchPicksInArea 에서 동시에 호출을 해도, 각 코루틴이 별개의 latestNearPick을 사용하기 때문에 동시에 접근하지 않을 것 같아요!

제가 생각했던건 MapViewModel파일의 fetchPicksInBounds내부의 mutex 필요성인데 여기서는 _picks나 clusterer를 업데이트할 때 민주님이 답변해주신대로 여러개의 값을 업데이트되는 문제를 방지하기 위해 필요하다고 생각했었습니다.

그런데, 현재 fetchPicksInBounds를 호출하는 경우는 디바이스를 초기설정하는 경우와 LaunchedEffect내부에 있습니다.
공식 문서에서 LaunchedEffect의 키가 변경되면 기존 코루틴이 취소되고 새 코루틴에서 실행된다는 내용이 있네요..!

fetchPicksInBounds가 동시에 실행되는 경우가 없다면 mutex가 필요없지 않을까 라는 생각이 들었습니다.
직접 로그를 찍어보아도 각 코루틴이 종료되고 실행됨을 확인했습니다.

혹시 위 내용도 이상한 점이 있다면 말씀해주시면 감사하겠습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

지난 회의 이후 깨달았습니다! 뮤텍스는 필요 없는 것 같네요 반영해두었습니다

저는 데이터 소스의 fetchPicksInArea()에서 여러 개의 Flow를 방출하는 줄 알았습니다 ㅎㅎ... trySend()로 한 channel에 데이터를 계속해서 보내는 거라 결국은 하나의 Flow에서 계속해서 데이터를 방출하는 거였네요

@meanjoo
Copy link
Collaborator Author

meanjoo commented Apr 6, 2025

지난 회의에서 Flow가 종료되면 이걸 처리하는 데가 없어서 앱이 꺼지는 문제가 있다고 이야기해주신 부분 처리해서 올려뒀습니다. 일반적으로 테스트할 때는 try-catch 구문에서 catch로 빠지는 경우가 없어서 close(e)가 되었을 때의 처리를 잘 해두지는 않았던 것 같습니다. 시뮬레이션으로는 어떻게 해야 catch 구문으로 빠지는지 잘 모르겠어서 코드 상 일부러 오류를 발생시켜 테스트하긴 했습니다.

예외 처리를 데이터 소스에서 예외 발생 시 그 이유를 Repository로 전달 -> Repository에서 이유에 따라 에러를 발생 -> 뷰모델에서 에러에 따라 상태 처리 이러한 흐름으로 처리하는 방법을 처음에는 생각했었는데, 현재 부분은 에러 정보가 그렇게 중요한 건 아니라고 생각합니다. 데이터를 불러오는 과정에서 발생한 문제를 굳이 이유에 따라 분리할 필요가 없다고 생각했습니다. 따라서 데이터 소스나 Repository에서 특별한 처리를 하지는 않고 뷰모델에서 catch로 에러를 잡고 토스트 메시지를 띄우도록 해뒀는데 어떤지 봐주시면 감사하겠습니다!

Screen_recording_20250407_032539.webm

@meanjoo meanjoo requested review from yuni-ju and miller198 April 6, 2025 18:32
Copy link
Collaborator

@yuni-ju yuni-ju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋습니다!! 😁

Copy link
Collaborator

@miller198 miller198 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨어요! 주석이나 로그 정리 + develop 머지 한번 해주세요!!

@meanjoo meanjoo merged commit 9bad71b into develop Apr 28, 2025
@meanjoo meanjoo deleted the feature/#173-real-time-pick branch April 28, 2025 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 기능 개발
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[메인 지도] 픽 실시간 업데이트
3 participants