You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/09/17 17:48:54 UTC

[GitHub] [kafka] fvaleri opened a new pull request, #12656: Update offset.storage.topic description

fvaleri opened a new pull request, #12656:
URL: https://github.com/apache/kafka/pull/12656

   This is a minor change just to clarify that this topic only stores offsets of source connector tasks. Instead, offsets for sink connector tasks are stored in __consumer_offsets, as for consumer groups.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] fvaleri commented on pull request #12656: MINOR: Update offset.storage.topic description

Posted by GitBox <gi...@apache.org>.
fvaleri commented on PR #12656:
URL: https://github.com/apache/kafka/pull/12656#issuecomment-1250780273

   @mimaison 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] C0urante commented on pull request #12656: MINOR: Update offset.storage.topic description

Posted by GitBox <gi...@apache.org>.
C0urante commented on PR #12656:
URL: https://github.com/apache/kafka/pull/12656#issuecomment-1250915379

   Thanks @fvaleri, this is a useful change. I've left a few small comments but it should be good to go after they're addressed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] C0urante commented on a diff in pull request #12656: MINOR: Update offset.storage.topic description

Posted by GitBox <gi...@apache.org>.
C0urante commented on code in PR #12656:
URL: https://github.com/apache/kafka/pull/12656#discussion_r974156910


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java:
##########
@@ -108,7 +108,7 @@ public class WorkerConfig extends AbstractConfig {
 
     public static final String OFFSET_COMMIT_INTERVAL_MS_CONFIG = "offset.flush.interval.ms";
     private static final String OFFSET_COMMIT_INTERVAL_MS_DOC
-            = "Interval at which to try committing offsets for tasks.";
+            = "Interval at which to try committing offsets for source tasks.";

Review Comment:
   The offset commit interval also affects sink tasks; see here:
   https://github.com/apache/kafka/blob/352c71ffb5d825c4a88454c12b9fa66c1750add3/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerSinkTask.java#L212



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] C0urante commented on a diff in pull request #12656: MINOR: Update offset.storage.topic description

Posted by GitBox <gi...@apache.org>.
C0urante commented on code in PR #12656:
URL: https://github.com/apache/kafka/pull/12656#discussion_r974158739


##########
docs/connect.html:
##########
@@ -53,7 +53,7 @@ <h4><a id="connect_running" href="#connect_running">Running Kafka Connect</a></h
 
     <p>The important configuration options specific to standalone mode are:</p>
     <ul>
-        <li><code>offset.storage.file.filename</code> - File to store offset data in</li>
+        <li><code>offset.storage.file.filename</code> - File to store source task offsets</li>

Review Comment:
   Same suggestion RE "connector" vs. "task":
   ```suggestion
           <li><code>offset.storage.file.filename</code> - File to store source connector offsets</li>
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] fvaleri commented on pull request #12656: MINOR: Update offset.storage.topic description

Posted by GitBox <gi...@apache.org>.
fvaleri commented on PR #12656:
URL: https://github.com/apache/kafka/pull/12656#issuecomment-1251149885

   Thanks @C0urante for the quick feedback. It should be good now.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] fvaleri commented on a diff in pull request #12656: MINOR: Update offset.storage.topic description

Posted by GitBox <gi...@apache.org>.
fvaleri commented on code in PR #12656:
URL: https://github.com/apache/kafka/pull/12656#discussion_r974366092


##########
docs/connect.html:
##########
@@ -53,7 +53,7 @@ <h4><a id="connect_running" href="#connect_running">Running Kafka Connect</a></h
 
     <p>The important configuration options specific to standalone mode are:</p>
     <ul>
-        <li><code>offset.storage.file.filename</code> - File to store offset data in</li>
+        <li><code>offset.storage.file.filename</code> - File to store source task offsets</li>

Review Comment:
   Thanks.



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/standalone/StandaloneConfig.java:
##########
@@ -28,7 +28,7 @@ public class StandaloneConfig extends WorkerConfig {
      * <code>offset.storage.file.filename</code>
      */
     public static final String OFFSET_STORAGE_FILE_FILENAME_CONFIG = "offset.storage.file.filename";
-    private static final String OFFSET_STORAGE_FILE_FILENAME_DOC = "File to store offset data in";
+    private static final String OFFSET_STORAGE_FILE_FILENAME_DOC = "File to store source task offsets";

Review Comment:
   Yes, I like consistency. Thanks.



##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java:
##########
@@ -108,7 +108,7 @@ public class WorkerConfig extends AbstractConfig {
 
     public static final String OFFSET_COMMIT_INTERVAL_MS_CONFIG = "offset.flush.interval.ms";
     private static final String OFFSET_COMMIT_INTERVAL_MS_DOC
-            = "Interval at which to try committing offsets for tasks.";
+            = "Interval at which to try committing offsets for source tasks.";

Review Comment:
   Right, thanks.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] C0urante commented on a diff in pull request #12656: MINOR: Update offset.storage.topic description

Posted by GitBox <gi...@apache.org>.
C0urante commented on code in PR #12656:
URL: https://github.com/apache/kafka/pull/12656#discussion_r974156910


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java:
##########
@@ -108,7 +108,7 @@ public class WorkerConfig extends AbstractConfig {
 
     public static final String OFFSET_COMMIT_INTERVAL_MS_CONFIG = "offset.flush.interval.ms";
     private static final String OFFSET_COMMIT_INTERVAL_MS_DOC
-            = "Interval at which to try committing offsets for tasks.";
+            = "Interval at which to try committing offsets for source tasks.";

Review Comment:
   The offset commit interval also affects sink tasks; see here:
   https://github.com/apache/kafka/blob/352c71ffb5d825c4a88454c12b9fa66c1750add3/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerSinkTask.java#L212
   We can probably revert this change.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] C0urante merged pull request #12656: MINOR: Update offset.storage.topic description

Posted by GitBox <gi...@apache.org>.
C0urante merged PR #12656:
URL: https://github.com/apache/kafka/pull/12656


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] C0urante commented on a diff in pull request #12656: MINOR: Update offset.storage.topic description

Posted by GitBox <gi...@apache.org>.
C0urante commented on code in PR #12656:
URL: https://github.com/apache/kafka/pull/12656#discussion_r974158439


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/standalone/StandaloneConfig.java:
##########
@@ -28,7 +28,7 @@ public class StandaloneConfig extends WorkerConfig {
      * <code>offset.storage.file.filename</code>
      */
     public static final String OFFSET_STORAGE_FILE_FILENAME_CONFIG = "offset.storage.file.filename";
-    private static final String OFFSET_STORAGE_FILE_FILENAME_DOC = "File to store offset data in";
+    private static final String OFFSET_STORAGE_FILE_FILENAME_DOC = "File to store source task offsets";

Review Comment:
   To remain consistent with the description for the `offset.storage.topic`, we should probably use the term "connector" instead of "task":
   ```suggestion
       private static final String OFFSET_STORAGE_FILE_FILENAME_DOC = "File to store source connector offsets";
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org