Skip to content

Fix/control kafka mqtt block - #103

Merged
ION127 merged 23 commits into
mainfrom
fix/control-kafka-mqtt-block
Jun 26, 2026
Merged

Fix/control kafka mqtt block#103
ION127 merged 23 commits into
mainfrom
fix/control-kafka-mqtt-block

Conversation

@ION127

@ION127 ION127 commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

ION127 added 21 commits June 19, 2026 16:29
- CompletableFuture.runAsync 제출 실패 시 outer try-catch 추가 (EventService, RelocationController)
- RelocationController: RejectedExecutionException 발생 시 503 반환
- RestClient.Builder 빈 주입 방식으로 변경 (RelocationServiceClient, ControlServiceClient)
- 좌표 키 'lng' → 'lon' 통일 (RelocationTriggerService)
- DispatchServiceApplicationTests: Redis/Kafka 자동설정 제외 방식으로 변경
- SaveWeightServiceApplicationTests: test yml 기반으로 단순화
- DispatchServiceApplicationTests: spring.autoconfigure.exclude 제거 → test yml이 Redis/Kafka 커버
- RestClient.Builder.clone() 추가 (RelocationServiceClient, ControlServiceClient) — 공유 빌더 변경 부작용 방지
- RelocationTriggerService: 좌표 쌍 처리 chunked(2) 방식으로 가독성 개선
- KafkaConsumerConfig: concurrency를 KAFKA_DISPATCH_CONCURRENCY 환경변수로 제어 (기본값 4)
- application.yml: staleness-threshold-seconds, kafka.dispatch.concurrency 환경변수 바인딩 추가
- 기본값: staleness 300초(5분), concurrency 4 (파티션 수와 일치)
block-internal-api에서 /dispatch/vehicles/*/active 제거 후
인증 없는 전용 어드민 라우트(dispatch-service-admin-vehicles-active)를
추가하여 baro-admin의 배차 정보 조회 복구
- TokenService: JWT어드민 role claim 포함
- AuthService: 토큰 발급 시 user.role 전달
- JwtAuthenticationGatewayFilterFactory: requiredRole Config 추가, FORBIDDEN 처리
- GatewayAuthenticationHeaders: X-Authenticated-Role 헤더 추가
- application.yml: dispatch/vehicles/*/active에 JwtAuthentication=ADMIN 적용
findAll() → findAll(pageable) 변경, @PageableDefault(size=500)으로
최대 반환 건수 제한. DoS 및 메모리 exhaustion 방지.
max.block.ms=0, retries=0 설정으로 Kafka 장애 시 kafkaTemplate.send() 즉시 실패.
기존 default max.block.ms=60000ms → MQTT 수신 스레드가 60초 블로킹 →
mosquitto 큐 overflow → Broken pipe → admin 차량 1대만 표시되는 문제 수정.
Kafka publish 실패 로그 30초 rate limit 적용.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new taxi stand retrieval endpoint in the dispatch service, including its JPA entity, repository, and gateway routing. It also configures the Kafka producer in the control service to prevent blocking and adds rate-limited logging for publish failures. Feedback highlights critical issues: setting MAX_BLOCK_MS_CONFIG to 0 may cause immediate timeouts, a race condition exists in the logging rate-limiter, Kotlin JPA entities should use mutable fields (var), returning Spring Data's Page directly exposes internal metadata, and the new gateway route lacks authentication.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +27 to +28
ProducerConfig.MAX_BLOCK_MS_CONFIG to 0,
ProducerConfig.RETRIES_CONFIG to 0,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Setting ProducerConfig.MAX_BLOCK_MS_CONFIG to 0 will cause immediate TimeoutExceptions (e.g., Failed to update metadata after 0 ms) during the very first send() call or during metadata refreshes, because fetching metadata from the broker requires a non-zero network round-trip time.

To prevent blocking the MQTT thread without causing spurious failures, consider setting MAX_BLOCK_MS_CONFIG to a small but reasonable value (e.g., 500 ms). Alternatively, offload the Kafka publish operation to a separate thread pool or use Kotlin Coroutines (Dispatchers.IO) so that the MQTT thread is never blocked regardless of Kafka's blocking behavior.

Suggested change
ProducerConfig.MAX_BLOCK_MS_CONFIG to 0,
ProducerConfig.RETRIES_CONFIG to 0,
ProducerConfig.MAX_BLOCK_MS_CONFIG to 500,
ProducerConfig.RETRIES_CONFIG to 0,

Comment on lines +94 to +103
private fun logKafkaFailure(message: String?) {
val count = kafkaFailCount.incrementAndGet()
val now = System.currentTimeMillis()
val last = kafkaFailLoggedAt.get()
// 최초 실패 또는 30초마다 한 번만 로그
if (count == 1L || now - last > 30_000) {
kafkaFailLoggedAt.set(now)
log.warn("Kafka publish 실패 (누적 {}건): {}", count, message)
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Under high concurrency (e.g., when Kafka is down and multiple MQTT threads fail to publish simultaneously), there is a race condition between kafkaFailLoggedAt.get() and kafkaFailLoggedAt.set(now). Multiple threads can pass the if condition and log duplicate warnings.

Using compareAndSet ensures that only one thread successfully updates the timestamp and logs the warning, effectively rate-limiting the logs as intended.

Suggested change
private fun logKafkaFailure(message: String?) {
val count = kafkaFailCount.incrementAndGet()
val now = System.currentTimeMillis()
val last = kafkaFailLoggedAt.get()
// 최초 실패 또는 30초마다 한 번만 로그
if (count == 1L || now - last > 30_000) {
kafkaFailLoggedAt.set(now)
log.warn("Kafka publish 실패 (누적 {}건): {}", count, message)
}
}
private fun logKafkaFailure(message: String?) {
val count = kafkaFailCount.incrementAndGet()
val now = System.currentTimeMillis()
val last = kafkaFailLoggedAt.get()
if (count == 1L || now - last > 30_000) {
if (kafkaFailLoggedAt.compareAndSet(last, now)) {
log.warn("Kafka publish 실패 (누적 {}건): {}", count, message)
}
}
}

Comment on lines +10 to +32
class TaxiStandEntity(
@Id
@Column(name = "id")
val id: String = "",

@Column(name = "longitude", nullable = false)
val longitude: Double = 0.0,

@Column(name = "latitude", nullable = false)
val latitude: Double = 0.0,

@Column(name = "district")
val district: String? = null,

@Column(name = "road_address")
val roadAddress: String? = null,

@Column(name = "extra_road")
val extraRoad: String? = null,

@Column(name = "name")
val name: String? = null,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

In Kotlin, JPA entities should ideally use var instead of val for fields mapped to database columns (except possibly the @Id field if it's immutable).

Hibernate relies on mutability for proxying, lazy loading, and dirty checking. Using val makes the fields immutable, which can cause issues if Hibernate needs to instantiate proxies or if you need to update these fields in the future.

Additionally, ensure that the Kotlin allopen and noarg compiler plugins are configured in your build file to allow Hibernate to subclass and instantiate the entity correctly.

class TaxiStandEntity(
    @Id
    @Column(name = "id")
    val id: String = "",

    @Column(name = "longitude", nullable = false)
    var longitude: Double = 0.0,

    @Column(name = "latitude", nullable = false)
    var latitude: Double = 0.0,

    @Column(name = "district")
    var district: String? = null,

    @Column(name = "road_address")
    var roadAddress: String? = null,

    @Column(name = "extra_road")
    var extraRoad: String? = null,

    @Column(name = "name")
    var name: String? = null,
)

Comment on lines +27 to +36
): Page<TaxiStandResponse> =
taxiStandRepository.findAll(pageable).map {
TaxiStandResponse(
id = it.id,
longitude = it.longitude,
latitude = it.latitude,
name = it.name,
district = it.district,
)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Returning Spring Data's Page object directly from the controller exposes internal pagination metadata (such as pageable, sort, first, last, etc.) to the API clients. This tightly couples your API contract to Spring Data's implementation details.

Consider wrapping the paginated result in a custom API response DTO that only exposes the necessary pagination fields (e.g., content, pageNumber, pageSize, totalElements, totalPages).

Comment on lines +82 to +89
- id: dispatch-service-stands
uri: ${DISPATCH_SERVICE_URL:http://localhost:8082}
predicates:
- Method=GET
- Path=/dispatch/stands
filters:
- RemoveRequestHeader=Authorization
- CircuitBreaker=dispatchServiceCircuitBreaker

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

security-medium medium

The route dispatch-service-stands uses RemoveRequestHeader=Authorization and does not include the JwtAuthentication filter. This makes the /dispatch/stands endpoint publicly accessible without any authentication.

If taxi stand data is sensitive or should only be accessible to authenticated users, please ensure that JwtAuthentication is added to the filters list instead of removing the Authorization header.

ION127 added 2 commits June 26, 2026 11:25
- MAX_BLOCK_MS 0→500ms (metadata 조회 여유 확보)
- logKafkaFailure compareAndSet으로 race condition 수정
- TaxiStandEntity val→var (Hibernate proxy 호환)
- Page 직접 반환 → StandsPageResponse DTO로 래핑
@ION127
ION127 merged commit da08fd8 into main Jun 26, 2026
7 checks passed
@ION127
ION127 deleted the fix/control-kafka-mqtt-block branch June 26, 2026 02:28
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 this pull request may close these issues.

1 participant