KAFKA-9401: Reduce contention for Fetch requests #15836

gaurav-narula · 2024-04-30T18:25:29Z

KIP-227 introduced in-memory caching of FetchSessions. Brokers with a large number of Fetch requests suffer from contention on trying to acquire a lock on FetchSessionCache.

This change aims to reduce lock contention for FetchSessionCache by sharding the cache into multiple segments, each responsible for an equal range of sessionIds. Assuming Fetch requests have a uniform distribution of sessionIds, the probability of contention on a segment is reduced by a factor of the number of segments.

We ensure backwards compatibility by ensuring total number of cache entries remain the same as configured and sessionIds are randomly allocated.

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

gaurav-narula · 2024-04-30T18:33:31Z

The following images show lock profiles collected using async-profiler before and after this change with numCacheShards = 8 profiling time = 30s and demonstrates significant reduction in contention

Before

After

Edit: Corrected numCacheShards = 8 (was incorrectly mentioned as 64 before)

chia7712

@gaurav-narula thanks for this improvement. two major comments left. PTAL

chia7712 · 2024-04-30T20:28:06Z

core/src/main/scala/kafka/server/FetchSession.scala

-  metricsGroup.newGauge(FetchSession.NUM_INCREMENTAL_FETCH_PARTITIONS_CACHED, () => FetchSessionCache.this.totalPartitions)
-  metricsGroup.removeMetric(FetchSession.INCREMENTAL_FETCH_SESSIONS_EVICTIONS_PER_SEC)
+  metricsGroup.removeMetric(FetchSession.NUM_INCREMENTAL_FETCH_SESSIONS, metricTag)
+  metricsGroup.newGauge(FetchSession.NUM_INCREMENTAL_FETCH_SESSIONS, () => FetchSessionCache.this.size, metricTag)


not sure whether this is allowed. It seems to break the compatibility of metrics as it adds new tags. It means kafka users who monitoring this metrics need to update the query.

Am maybe wrong here but don't change tags of an existing metrics need a KIP!

That's a good point. The reason I added the labels was because of the removeMetric calls. On further investigation, it seems that metricsGroup.removeMetric isn't really needed and we can have a combined metric for all the shards. The lambda that calculates the value for the gauges needs to synchronize on the cache but it's a small overhead overall as this is only when metrics are gathered.

Yes, the metric would be KIP-able. But you have decided you don't need the tag :)

chia7712 · 2024-04-30T20:28:51Z

core/src/main/scala/kafka/server/BrokerServer.scala

-          KafkaServer.MIN_INCREMENTAL_FETCH_SESSION_EVICTION_MS))
+      // The FetchSessionCache is divided into config.numIoThreads shards, each responsible
+      // for sessionIds falling in [Max(1, shardNum * sessionIdRange), (shardNum + 1) * sessionIdRange)
+      val sessionIdRange = Int.MaxValue / config.numIoThreads


pardon me. what happens when users update numIoThreads dynamically?

Using config.numIoThreads is a heuristic and I reckon it's okay if shard count isn't modified dynamically along with numIoThreads.

In fact, I realised my "After" benchmark was with 8 shards and not 64 as I had a misconfiguration while running it which goes to show even some sharding goes a long way. How about we introduce a constant for now and set it to 8?

Since the number of shards need not depend on the thread pool size, I agree it may be less confusing to keep config.numIoThreads out of this.

1 on keeping number of threads out this.

How about we introduce a constant for now and set it to 8?

Out of curiosity are you planning later to make this constant a configuration?

Out of curiosity are you planning later to make this constant a configuration?

I think it's unlikely one would need to tweak this often. My experiment ran on a fairly busy cluster with ~50k Fetch rps (kafka.network-RequestMetrics-RequestsPerSec(request=Fetch)) on the broker which is reasonably high.

I would tend not to add a configuration for this. The value you're talking about sounds like it's doing the job on a busy workload, and it's small enough that there's negligible benefit of configuring it smaller for a tiny cluster. Having a configuration kind of crystallizes this aspect of the internal design of Kafka, and you might have an even better idea in the future that would make this configuration pointless.

OmniaGM

Thanks for the PR. I left few comments but I echo the same comments @chia7712 mentioned.

OmniaGM · 2024-05-01T09:55:38Z

core/src/main/scala/kafka/server/BrokerServer.scala

+      // The FetchSessionCache is divided into config.numIoThreads shards, each responsible
+      // for sessionIds falling in [Max(1, shardNum * sessionIdRange), (shardNum + 1) * sessionIdRange)
+      val sessionIdRange = Int.MaxValue / config.numIoThreads
+      val fetchSessionCaches = Range(0, config.numIoThreads)


0 to config.numIoThreads is usually more preferred to scala than Range

And same as @chia7712 comment before config.numIoThreads is listed as broker dynamic config so what would happened when this change.

OmniaGM · 2024-05-01T10:00:53Z

core/src/main/scala/kafka/server/FetchSession.scala

@@ -430,6 +439,9 @@ class FullFetchContext(private val time: Time,
      }
      cachedPartitions
    }
+    // We select a shard randomly out of the available options
+    val shard = ThreadLocalRandom.current().nextInt(caches.size)
+    val cache = caches.apply(shard);


you don't need apply, caches(shard) is enough

OmniaGM · 2024-05-01T10:45:56Z

core/src/main/scala/kafka/server/FetchSession.scala

+
+  def getShardedCache(sessionId: Int): FetchSessionCache = {
+    val shard = sessionId / caches.head.sessionIdRange
+    caches.apply(shard)


same here about apply

OmniaGM · 2024-05-01T10:46:57Z

core/src/main/scala/kafka/server/KafkaServer.scala

+        // The FetchSessionCache is divided into config.numIoThreads shards, each responsible
+        // for sessionIds falling in [Max(1, shardNum * sessionIdRange), (shardNum + 1) * sessionIdRange)
+        val sessionIdRange = Int.MaxValue / config.numIoThreads
+        val fetchSessionCaches = Range(0, config.numIoThreads)


same comment about Range vs to

I've used until because I needed an exclusive Range

OmniaGM · 2024-05-01T10:47:30Z

core/src/test/scala/unit/kafka/server/FetchSessionTest.scala

+    // Given
+    val time = new MockTime()
+    val sessionIdRange = Int.MaxValue / 8
+    val caches = Range(0, 8).map(shardNum => new FetchSessionCache(10, 1000, sessionIdRange, shardNum))


same as above

AndrewJSchofield

Thanks for the PR. It's good that you're improving this area.

AndrewJSchofield · 2024-05-03T15:08:06Z

core/src/main/scala/kafka/server/BrokerServer.scala

-          KafkaServer.MIN_INCREMENTAL_FETCH_SESSION_EVICTION_MS))
+      // The FetchSessionCache is divided into config.numIoThreads shards, each responsible
+      // for sessionIds falling in [Max(1, shardNum * sessionIdRange), (shardNum + 1) * sessionIdRange)
+      val sessionIdRange = Int.MaxValue / config.numIoThreads


I would tend not to add a configuration for this. The value you're talking about sounds like it's doing the job on a busy workload, and it's small enough that there's negligible benefit of configuring it smaller for a tiny cluster. Having a configuration kind of crystallizes this aspect of the internal design of Kafka, and you might have an even better idea in the future that would make this configuration pointless.

AndrewJSchofield · 2024-05-03T15:09:57Z

core/src/main/scala/kafka/server/FetchSession.scala

-  metricsGroup.newGauge(FetchSession.NUM_INCREMENTAL_FETCH_PARTITIONS_CACHED, () => FetchSessionCache.this.totalPartitions)
-  metricsGroup.removeMetric(FetchSession.INCREMENTAL_FETCH_SESSIONS_EVICTIONS_PER_SEC)
+  metricsGroup.removeMetric(FetchSession.NUM_INCREMENTAL_FETCH_SESSIONS, metricTag)
+  metricsGroup.newGauge(FetchSession.NUM_INCREMENTAL_FETCH_SESSIONS, () => FetchSessionCache.this.size, metricTag)


Yes, the metric would be KIP-able. But you have decided you don't need the tag :)

AndrewJSchofield · 2024-05-03T15:13:56Z

core/src/main/scala/kafka/server/FetchSession.scala

@@ -583,9 +595,13 @@ case class EvictableKey(privileged: Boolean, size: Int, id: Int) extends Compara
  *
  * @param maxEntries The maximum number of entries that can be in the cache.
  * @param evictionMs The minimum time that an entry must be unused in order to be evictable.
-  */
+  * @param sessionIdRange The number of sessionIds each cache shard handles. The range for a given shard is [Math.max(1, shardNum * sessionIdRange), (shardNum + 1) * sessionIdRange).


I know the [ , ) notation means what you intend for inclusive/exclusive bounds, but the presence of the parentheses makes it a bit hard to read I think. Maybe using >= and < would be clearer.

AndrewJSchofield · 2024-05-03T15:16:51Z

core/src/main/scala/kafka/server/FetchSession.scala

  * @param reqMetadata        The request metadata.
  * @param fetchData          The partition data from the fetch request.
  * @param usesTopicIds       True if this session should use topic IDs.
  * @param isFromFollower     True if this fetch request came from a follower.
  */
 class FullFetchContext(private val time: Time,
-                       private val cache: FetchSessionCache,
+                       private val caches: Seq[FetchSessionCache],


cacheShards?

AndrewJSchofield · 2024-05-03T15:18:51Z

core/src/main/scala/kafka/server/FetchSession.scala

-  */
+  * @param sessionIdRange The number of sessionIds each cache shard handles. The range for a given shard is [Math.max(1, shardNum * sessionIdRange), (shardNum + 1) * sessionIdRange).
+  * @param shardNum Identifier for this shard.
+ */
 class FetchSessionCache(private val maxEntries: Int,


Maybe the class name ought to be FetchSessionCache shard too. The cache really is the whole set of shards.

Thanks for the suggestion! I've renamed the existing type to FetchSessionCacheShard and FetchSessionCache is now essentially a wrapper around Seq[FetchSessionCacheShard]. This conveys the intention clearly indeed.

AndrewJSchofield · 2024-05-03T15:20:20Z

core/src/main/scala/kafka/server/FetchSession.scala

+
+  def this(time: Time, cache: FetchSessionCache) = this(time, Seq(cache))
+
+  def getShardedCache(sessionId: Int): FetchSessionCache = {


getCacheShard?

chia7712

@gaurav-narula nice improvement. a couple of comments left. PTAL

chia7712 · 2024-05-04T00:11:30Z

core/src/main/scala/kafka/server/FetchSession.scala

@@ -430,7 +438,10 @@ class FullFetchContext(private val time: Time,
      }
      cachedPartitions
    }
-    val responseSessionId = cache.maybeCreateSession(time.milliseconds(), isFromFollower,
+    // We select a shard randomly out of the available options
+    val shard = ThreadLocalRandom.current().nextInt(cache.size())


It seems both size and apply are used to randomly pick up a FetchSessionCacheShard. Maybe we can add a method getRandomCacheShard to FetchSessionCache.

chia7712 · 2024-05-04T01:12:22Z

core/src/main/scala/kafka/server/FetchSession.scala

+class FetchSessionCacheShard(private val maxEntries: Int,
+                             private val evictionMs: Long,
+                             val sessionIdRange: Int = Int.MaxValue,
+                             private val shardNum: Int = 0) extends Logging {


Should FetchSessionCacheShard define the logIdent to helper users distinguish the source of log message? For example, LogClean uses thread id to be the logIdent

kafka/core/src/main/scala/kafka/log/LogCleaner.scala

Line 559 in 1fd3915

this.logIdent = s"Cleaner $id: "

chia7712 · 2024-05-04T01:39:30Z

core/src/main/scala/kafka/server/FetchSession.scala

-    val responseSessionId = cache.maybeCreateSession(time.milliseconds(), isFromFollower,
+    // We select a shard randomly out of the available options
+    val shard = ThreadLocalRandom.current().nextInt(cache.size())
+    val cacheShard = cache(shard);


please remove ;

chia7712 · 2024-05-04T02:21:03Z

core/src/main/scala/kafka/server/BrokerServer.scala

+      val sessionIdRange = Int.MaxValue / NumFetchSessionCacheShards
+      val fetchSessionCacheShards = (0 until NumFetchSessionCacheShards)
+        .map(shardNum => new FetchSessionCacheShard(
+          config.maxIncrementalFetchSessionCacheSlots / NumFetchSessionCacheShards,


The behavior gets changed a bit here. max.incremental.fetch.session.cache.slots is used to control the "total" number of cached session. With this change, however, the session room could be full even though the "total" size does not reach the threshold defined by max.incremental.fetch.session.cache.slots.

This is not big issue, and we can update the docs of max.incremental.fetch.session.cache.slots for that behavior change.

That's a great point and it's quite subtle. I reckon this may happen because the cacheShards are picked randomly and it can be avoided by picking shards in round-robin. I'll make this change along with addressing the other comments 👍

Some subtle differences cannot be avoided, particularly around eviction. The KIP considers all existing sessions when considering a session for eviction while this change would consider only existing sessions within a shard for eviction. I'll update the documentation to call out the difference.

chia7712

@gaurav-narula thanks for updated PR. Please take a look at following comments.

chia7712 · 2024-05-04T18:39:57Z

core/src/main/scala/kafka/server/FetchSession.scala

@@ -690,6 +702,10 @@ class FetchSessionCache(private val maxEntries: Int,
    * 2. B is considered "stale" because it has been inactive for a long time, or
    * 3. A contains more partitions than B, and B is not recently created.
    *
+    * Prior to KAFKA-9401, the session cache was not sharded and we looked at all


This docs is great. Could you please update this also?

kafka/core/src/main/scala/kafka/server/KafkaConfig.scala

Line 182 in 9b8aac2

val MaxIncrementalFetchSessionCacheSlotsDoc = "The maximum number of incremental fetch sessions that we will maintain."

chia7712 · 2024-05-04T18:46:10Z

core/src/main/scala/kafka/server/FetchSession.scala

+
+  // Returns the shard in round-robin
+  def getNextCacheShard: FetchSessionCacheShard = {
+    val shardNum = (FetchSessionCache.counter.getAndIncrement() % size).toInt


As int is enough to this case, maybe we can use AtomicInteger?

I used AtomicLong to practically rule out an overflow but found Utils.toPositive which is used by RoundRobinPartitioner :) Updated to use an AtomicInteger and also added some test to ensure round-robin allocations.

chia7712 · 2024-05-04T18:56:29Z

core/src/main/scala/kafka/server/FetchSession.scala

+  FetchSessionCache.metricsGroup.newGauge(FetchSession.NUM_INCREMENTAL_FETCH_PARTITIONS_CACHED, () => cacheShards.map(_.totalPartitions).sum)
+
+  def getCacheShard(sessionId: Int): FetchSessionCacheShard = {
+    val shard = sessionId / cacheShards.head.sessionIdRange


It assumes the cacheShards is sorted by the shardNum, right? If so, could you please add comments for it?

chia7712

LGTM

gaurav-narula · 2024-05-06T16:00:40Z

Resolved conflict with trunk

chia7712 · 2024-05-07T00:34:39Z

@gaurav-narula Could you please rebase code to trigger QA again? It seems we have thread leaks in some tests. That is unrelated to this PR as I had observed that before.

KIP-227 introduced in-memory caching of FetchSessions. Brokers with a large number of Fetch requests suffer from contention on trying to acquire a lock on FetchSessionCache. This change aims to reduce lock contention for FetchSessionCache by sharding the cache into multiple segments, each responsible for an equal range of sessionIds. Assuming Fetch requests have a uniform distribution of sessionIds, the probability of contention on a segment is reduced by a factor of the number of segments. We ensure backwards compatibility by ensuring total number of cache entries remain the same as configured and sessionIds are randomly allocated.

Uses AtomicInteger after ensuring we don't overflow. Updated comments as suggested in review comments. Added tests for ensuring round-robin allocation.

soarez · 2024-05-07T17:05:36Z

:core:test timed out only on JDK 8 and Scala 2.12. Restarted the build

chia7712 · 2024-05-09T03:23:24Z

@gaurav-narula Could you take a look at those failed tests? I feel they are unrelated to this PR, so +1

soarez

Checked all failed tests, and they all pass locally. If @gaurav-narula can also confirm they're unrelated to the changes I can merge this.

gaurav-narula · 2024-05-11T12:51:19Z

Thanks for the review! I'm fairly convinced these failures are unrelated. The report on Github enterprise suggests the failed tests are flakey. Please refer the following links for the flakiness report.

https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=org.apache.kafka.connect.integration.OffsetsApiIntegrationTest&tests.test=testAlterSinkConnectorOffsetsOverriddenConsumerGroupId
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=org.apache.kafka.trogdor.coordinator.CoordinatorTest&tests.test=testTaskRequestWithOldStartMsGetsUpdated()
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=org.apache.kafka.connect.integration.OffsetsApiIntegrationTest&tests.test=testAlterSinkConnectorOffsetsOverriddenConsumerGroupId
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=kafka.api.ConsumerBounceTest&tests.test=testSeekAndCommitWithBrokerFailures()
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=kafka.api.DelegationTokenEndToEndAuthorizationWithOwnerTest&tests.test=testProduceConsumeViaAssign(String)[1]
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=org.apache.kafka.connect.integration.OffsetsApiIntegrationTest&tests.test=testResetSinkConnectorOffsetsDifferentKafkaClusterTargeted
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=kafka.api.SaslSslConsumerTest&tests.test=testCoordinatorFailover(String, String)[1]
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=kafka.api.SaslSslConsumerTest&tests.test=testCoordinatorFailover(String, String)[2]
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=kafka.api.SaslSslConsumerTest&tests.test=testCoordinatorFailover(String, String)[3]
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=kafka.api.SaslSslConsumerTest&tests.test=testCoordinatorFailover(String, String)[4]
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=kafka.server.DelegationTokenRequestsTest&tests.test=testDelegationTokenRequests(String)[1]
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=org.apache.kafka.tools.GetOffsetShellTest&tests.test=testTopicPartitionsArgWithInternalIncluded()[1]
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=org.apache.kafka.tools.GetOffsetShellTest&tests.test=testTopicPartitionsArg()[1]
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=org.apache.kafka.tools.TopicCommandIntegrationTest&tests.test=testDescribeWithDescribeTopicPartitionsApi(String)[2]
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=org.apache.kafka.connect.mirror.integration.MirrorConnectorsIntegrationExactlyOnceTest&tests.test=testSyncTopicConfigs()
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=kafka.api.PlaintextConsumerAssignorsTest&tests.test=testRemoteAssignorRange(String%2C%20String)%5B1%5D
https://ge.apache.org/scans/tests?search.timeZoneId=Europe%2FLondon&tests.container=org.apache.kafka.tools.TopicCommandIntegrationTest&tests.test=testDescribeWithDescribeTopicPartitionsApi(String)[2]

chia7712 · 2024-05-11T15:21:54Z

@gaurav-narula Please file PR for branch 3.7 if you feel this one needs to be backport :)

KIP-227 introduced in-memory caching of FetchSessions. Brokers with a large number of Fetch requests suffer from contention on trying to acquire a lock on FetchSessionCache. This change aims to reduce lock contention for FetchSessionCache by sharding the cache into multiple segments, each responsible for an equal range of sessionIds. Assuming Fetch requests have a uniform distribution of sessionIds, the probability of contention on a segment is reduced by a factor of the number of segments. We ensure backwards compatibility by ensuring total number of cache entries remain the same as configured and sessionIds are randomly allocated. Reviewers: Igor Soarez <soarez@apple.com>, Chia-Ping Tsai <chia7712@gmail.com>

chia7712 reviewed Apr 30, 2024

View reviewed changes

OmniaGM reviewed May 1, 2024

View reviewed changes

gaurav-narula force-pushed the KAFKA-9401 branch from abf51a7 to 58744a4 Compare May 3, 2024 15:14

AndrewJSchofield suggested changes May 3, 2024

View reviewed changes

gaurav-narula force-pushed the KAFKA-9401 branch 2 times, most recently from 4a78018 to 1396c70 Compare May 3, 2024 20:19

chia7712 reviewed May 4, 2024

View reviewed changes

gaurav-narula force-pushed the KAFKA-9401 branch 2 times, most recently from 2b889d6 to 59f3eaa Compare May 4, 2024 16:34

chia7712 reviewed May 4, 2024

View reviewed changes

gaurav-narula force-pushed the KAFKA-9401 branch from 728e975 to bff261f Compare May 5, 2024 10:34

chia7712 approved these changes May 5, 2024

View reviewed changes

gaurav-narula force-pushed the KAFKA-9401 branch from bff261f to 6b21ffb Compare May 6, 2024 16:00

gaurav-narula added 5 commits May 7, 2024 09:40

Address review comments

4ec03e7

Refactoring

67825ff

Round robin to select next cache shard

9ab782c

Add tests, comments and use AtomicInteger

64b5f31

Uses AtomicInteger after ensuring we don't overflow. Updated comments as suggested in review comments. Added tests for ensuring round-robin allocation.

gaurav-narula force-pushed the KAFKA-9401 branch from 6b21ffb to 64b5f31 Compare May 7, 2024 08:41

gaurav-narula mentioned this pull request May 8, 2024

KAFKA-16688: Use helper method to shutdown ExecutorService #15886

Merged

3 tasks

soarez approved these changes May 9, 2024

View reviewed changes

chia7712 merged commit 47841e0 into apache:trunk May 11, 2024
1 check failed

gaurav-narula mentioned this pull request May 11, 2024

KAFKA-9401 Reduce contention for Fetch requests #15925

Closed


		def this(time: Time, cache: FetchSessionCache) = this(time, Seq(cache))

		def getShardedCache(sessionId: Int): FetchSessionCache = {

KAFKA-9401: Reduce contention for Fetch requests #15836

KAFKA-9401: Reduce contention for Fetch requests #15836

Conversation

gaurav-narula commented Apr 30, 2024

Committer Checklist (excluded from commit message)

gaurav-narula commented Apr 30, 2024 • edited Loading

chia7712 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OmniaGM left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OmniaGM May 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndrewJSchofield left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chia7712 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chia7712 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chia7712 left a comment

Choose a reason for hiding this comment

gaurav-narula commented May 6, 2024

chia7712 commented May 7, 2024 • edited Loading

soarez commented May 7, 2024

chia7712 commented May 9, 2024

soarez left a comment

Choose a reason for hiding this comment

gaurav-narula commented May 11, 2024

chia7712 commented May 11, 2024

gaurav-narula commented Apr 30, 2024 •

edited

Loading

OmniaGM May 1, 2024 •

edited

Loading

chia7712 commented May 7, 2024 •

edited

Loading