You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "polandll (via GitHub)" <gi...@apache.org> on 2023/01/25 20:39:51 UTC

[GitHub] [cassandra] polandll commented on a diff in pull request #2062: CASSANDRA-18185 Accumulate all `docs` PR

polandll commented on code in PR #2062:
URL: https://github.com/apache/cassandra/pull/2062#discussion_r1087130975


##########
doc/modules/cassandra/pages/architecture/dynamo.adoc:
##########
@@ -256,7 +255,7 @@ secondary indices with them.
 Transient replication is an experimental feature that is not ready
 for production use. The expected audience is experienced users of
 Cassandra capable of fully validating a deployment of their particular
-application. That means being able check that operations like reads,
+application. That means being able to check that operations like reads,

Review Comment:
   ```suggestion
   application. That means you have the experience to check that operations like reads,
   ```



##########
doc/modules/cassandra/pages/architecture/guarantees.adoc:
##########
@@ -97,12 +97,12 @@ uncommitted, without making a new addition or update.
 The guarantee for batched writes across multiple tables is that they
 will eventually succeed, or none will. Batch data is first written to
 batchlog system data, and when the batch data has been successfully
-stored in the cluster the batchlog data is removed. The batch is
-replicated to another node to ensure the full batch completes in the
-event the coordinator node fails.
+stored in the cluster, the batchlog data is removed. The batch is
+replicated to another node to ensure that the full batch completes in
+the event if the coordinator node fails.

Review Comment:
   ```suggestion
   the event the coordinator node fails.
   ```



##########
doc/modules/cassandra/pages/architecture/snitch.adoc:
##########
@@ -2,17 +2,17 @@
 
 In cassandra, the snitch has two functions:
 
-* it teaches Cassandra enough about your network topology to route
+* It teaches Cassandra enough about your network topology to route
 requests efficiently.
-* it allows Cassandra to spread replicas around your cluster to avoid
+* It allows Cassandra to spread replicas around your cluster to avoid

Review Comment:
   same comment



##########
doc/modules/cassandra/pages/cql/types.adoc:
##########
@@ -338,13 +338,13 @@ include::example$CQL/update_list.cql[]
 .Warning
 ====
 The append and prepend operations are not idempotent by nature. So in
-particular, if one of these operation timeout, then retrying the
+particular, if one of these operations timeout, then retrying the

Review Comment:
   ```suggestion
   particular, if one of these operations times out, then retrying the
   ```



##########
doc/modules/cassandra/pages/architecture/overview.adoc:
##########
@@ -59,19 +59,19 @@ keys.
 CQL supports numerous advanced features over a partitioned dataset such
 as:
 
-* Single partition lightweight transactions with atomic compare and set
-semantics.
+* Single-partition lightweight transactions with atomic compare and set
+semantics
 * User-defined types, functions and aggregates
-* Collection types including sets, maps, and lists.
+* Collection types including sets, maps, and lists
 * Local secondary indices
-* (Experimental) materialized views
+* (Experimental) materialized views.

Review Comment:
   ```suggestion
   * (Experimental) materialized views
   ```



##########
doc/modules/cassandra/pages/operating/auditlogging.adoc:
##########
@@ -186,52 +210,101 @@ excluded_categories: DDL, DML, QUERY, PREPARE
 Users to audit log are set with the `included_users` and `excluded_users` options. 
 The `included_users` option specifies a comma-separated list of users to include explicitly.
 The `excluded_users` option specifies a comma-separated list of users to exclude explicitly.
-By default all users are included, and no users are excluded. 
+By default, all users are included, and no users are excluded.
 
 [source, yaml]
 ----
 included_users: 
 excluded_users: john, mary
 ----
 
+[[roll_cycle]]
 === roll_cycle
 
 The ``roll_cycle`` defines the frequency with which the audit log segments are rolled.
-Supported values are ``HOURLY`` (default), ``MINUTELY``, and ``DAILY``.
+Supported values are:
+
+- ``MINUTELY``
+- ``FIVE_MINUTELY``
+- ``TEN_MINUTELY``
+- ``TWENTY_MINUTELY``
+- ``HALF_HOURLY``
+- ``HOURLY`` (default)
+- ``TWO_HOURLY``
+- ``FOUR_HOURLY``
+- ``SIX_HOURLY``
+- ``DAILY``
+
 For example: ``roll_cycle: DAILY``
 
+WARNING: Read the following paragraph when changing ``roll_cycle`` on a production node.
+
+With the `BinLogger` implementation, any attempt to modify the roll cycle on a node where audit logging was previously enabled will fail silentely due to https://github.com/OpenHFT/Chronicle-Queue[Chronicle Queue] roll cycle inference mechanism (even if you delete the ``metadata.cq4t`` file).
+
+Here is an example of such an override visible in Cassandra logs:
+----
+INFO  [main] <DATE TIME> BinLog.java:420 - Attempting to configure bin log: Path: /path/to/audit Roll cycle: TWO_HOURLY [...]
+WARN  [main] <DATE TIME> SingleChronicleQueueBuilder.java:477 - Overriding roll cycle from TWO_HOURLY to FIVE_MINUTE
+----
+
+In order to change ``roll_cycle`` on a node, you have to:
+
+1. Stop Cassandra
+2. Move or offload all audit logs somewhere else (in a safe and durable location)
+3. Restart Cassandra.
+4. Check Cassandra logs
+5. Make sure that audit log filenames under ``audit_logs_dir`` correspond to the new roll cycle.
+
 === block
 
 The ``block`` option specifies whether audit logging should block writing or drop log records if the audit logging falls behind. Supported boolean values are ``true`` (default) or ``false``.
-For example: ``block: false`` to drop records
+
+For example: ``block: false`` to drop records (e.g. if audit is used for troobleshooting)
+
+For regulatory compliance purpose, it's a good practice to explicitely set ``block: true`` to prevent any regression in case of future default value change.
 
 === max_queue_weight
 
 The ``max_queue_weight`` option sets the maximum weight of in-memory queue for records waiting to be written to the file before blocking or dropping.  The option must be set to a positive value. The default value is 268435456, or 256 MiB.
+
 For example, to change the default: ``max_queue_weight: 134217728 # 128 MiB``
 
 === max_log_size
 
 The ``max_log_size`` option sets the maximum size of the rolled files to retain on disk before deleting the oldest file.  The option must be set to a positive value. The default is 17179869184, or 16 GiB.
 For example, to change the default: ``max_log_size: 34359738368 # 32 GiB``
 
+WARNING: ``max_log_size`` is ignored if ``archive_command`` option is set.
+
+[[archive_command]]
 === archive_command
 
+NOTE: If ``archive_command`` option is empty or unset (default), Cassandra uses a built-in DeletingArchiver that deletes the oldest files if ``max_log_size`` is reached.
+
 The ``archive_command`` option sets the user-defined archive script to execute on rolled log files.
-For example: ``archive_command: /usr/local/bin/archiveit.sh %path # %path is the file being rolled``
+For example: ``archive_command: "/usr/local/bin/archiveit.sh %path"``
 
-=== max_archive_retries
+``%path`` is replaced with the absolute file path of the file being rolled.
 
-The ``max_archive_retries`` option sets the max number of retries of failed archive commands. The default is 10.
-For example: ``max_archive_retries: 10``
+When using a user-defined script, Cassandra do **not** use the DeletingArchiver, so it's the responsability of the script to make any required cleanup.

Review Comment:
   ```suggestion
   When using a user-defined script, Cassandra does **not** use the DeletingArchiver, so it's the responsibility of the script to make any required cleanup.
   ```



##########
doc/modules/cassandra/pages/operating/auditlogging.adoc:
##########
@@ -186,52 +210,101 @@ excluded_categories: DDL, DML, QUERY, PREPARE
 Users to audit log are set with the `included_users` and `excluded_users` options. 
 The `included_users` option specifies a comma-separated list of users to include explicitly.
 The `excluded_users` option specifies a comma-separated list of users to exclude explicitly.
-By default all users are included, and no users are excluded. 
+By default, all users are included, and no users are excluded.
 
 [source, yaml]
 ----
 included_users: 
 excluded_users: john, mary
 ----
 
+[[roll_cycle]]
 === roll_cycle
 
 The ``roll_cycle`` defines the frequency with which the audit log segments are rolled.
-Supported values are ``HOURLY`` (default), ``MINUTELY``, and ``DAILY``.
+Supported values are:
+
+- ``MINUTELY``
+- ``FIVE_MINUTELY``
+- ``TEN_MINUTELY``
+- ``TWENTY_MINUTELY``
+- ``HALF_HOURLY``
+- ``HOURLY`` (default)
+- ``TWO_HOURLY``
+- ``FOUR_HOURLY``
+- ``SIX_HOURLY``
+- ``DAILY``
+
 For example: ``roll_cycle: DAILY``
 
+WARNING: Read the following paragraph when changing ``roll_cycle`` on a production node.
+
+With the `BinLogger` implementation, any attempt to modify the roll cycle on a node where audit logging was previously enabled will fail silentely due to https://github.com/OpenHFT/Chronicle-Queue[Chronicle Queue] roll cycle inference mechanism (even if you delete the ``metadata.cq4t`` file).
+
+Here is an example of such an override visible in Cassandra logs:
+----
+INFO  [main] <DATE TIME> BinLog.java:420 - Attempting to configure bin log: Path: /path/to/audit Roll cycle: TWO_HOURLY [...]
+WARN  [main] <DATE TIME> SingleChronicleQueueBuilder.java:477 - Overriding roll cycle from TWO_HOURLY to FIVE_MINUTE
+----
+
+In order to change ``roll_cycle`` on a node, you have to:
+
+1. Stop Cassandra
+2. Move or offload all audit logs somewhere else (in a safe and durable location)
+3. Restart Cassandra.
+4. Check Cassandra logs
+5. Make sure that audit log filenames under ``audit_logs_dir`` correspond to the new roll cycle.
+
 === block
 
 The ``block`` option specifies whether audit logging should block writing or drop log records if the audit logging falls behind. Supported boolean values are ``true`` (default) or ``false``.
-For example: ``block: false`` to drop records
+
+For example: ``block: false`` to drop records (e.g. if audit is used for troobleshooting)
+
+For regulatory compliance purpose, it's a good practice to explicitely set ``block: true`` to prevent any regression in case of future default value change.
 
 === max_queue_weight
 
 The ``max_queue_weight`` option sets the maximum weight of in-memory queue for records waiting to be written to the file before blocking or dropping.  The option must be set to a positive value. The default value is 268435456, or 256 MiB.
+
 For example, to change the default: ``max_queue_weight: 134217728 # 128 MiB``
 
 === max_log_size
 
 The ``max_log_size`` option sets the maximum size of the rolled files to retain on disk before deleting the oldest file.  The option must be set to a positive value. The default is 17179869184, or 16 GiB.
 For example, to change the default: ``max_log_size: 34359738368 # 32 GiB``
 
+WARNING: ``max_log_size`` is ignored if ``archive_command`` option is set.
+
+[[archive_command]]
 === archive_command
 
+NOTE: If ``archive_command`` option is empty or unset (default), Cassandra uses a built-in DeletingArchiver that deletes the oldest files if ``max_log_size`` is reached.
+
 The ``archive_command`` option sets the user-defined archive script to execute on rolled log files.
-For example: ``archive_command: /usr/local/bin/archiveit.sh %path # %path is the file being rolled``
+For example: ``archive_command: "/usr/local/bin/archiveit.sh %path"``
 
-=== max_archive_retries
+``%path`` is replaced with the absolute file path of the file being rolled.
 
-The ``max_archive_retries`` option sets the max number of retries of failed archive commands. The default is 10.
-For example: ``max_archive_retries: 10``
+When using a user-defined script, Cassandra do **not** use the DeletingArchiver, so it's the responsability of the script to make any required cleanup.
 
+Cassandra will call the user-defined script as soon as the log file is rolled. It means that Chronicle Queue's QueueFileShrinkManager will not be able to shrink the sparse log file because it's done asynchronously. In other words, all log files will have at least the size of the default block size (80 MiB), even if there are only a few KB of real data. Consequently, some warnings will appear in Cassandra system.log:
 
-An audit log file could get rolled for other reasons as well such as a
-log file reaches the configured size threshold.
+----
+WARN  [main/queue~file~shrink~daemon] <DATE TIME> QueueFileShrinkManager.java:63 - Failed to shrink file as it exists no longer, file=/path/to/xxx.cq4
+----
 
-Audit logging can also be configured using ``nodetool` when enabling the feature, and will override any values set in the `cassandra.yaml` file, as discussed in the next section.
+TIP: Because Cassandra does not make use of Pretoucher, you can configure Chronicle Queue to shrink files synchronously -- i.e. as soon as the file is rolled -- with ``chronicle.queue.synchronousFileShrinking`` JVM properties. For instance, you can add the following line at the end of ``cassandra-env.sh``: ``JVM_OPTS="$JVM_OPTS -Dchronicle.queue.synchronousFileShrinking=true"``

Review Comment:
   ```suggestion
   TIP: 
   ====
   Because Cassandra does not make use of Pretoucher, you can configure Chronicle Queue to shrink files synchronously -- i.e. as soon as the file is rolled -- with `chronicle.queue.synchronousFileShrinking` JVM properties. For instance, you can add the following line at the end of `cassandra-env.sh`: `JVM_OPTS="$JVM_OPTS -Dchronicle.queue.synchronousFileShrinking=true"`
   ====



##########
doc/modules/cassandra/pages/architecture/storage_engine.adoc:
##########
@@ -221,5 +224,5 @@ match the "ib" SSTable version
 
 [source,bash]
 ----
-include:example$find_sstables.sh[]
+include::../../examples/BASH/find_sstables.sh[]

Review Comment:
   Never use a path in includes in asciidoc/antora. https://docs.antora.org/antora/latest/page/include-an-example/



##########
doc/modules/cassandra/pages/architecture/storage_engine.adoc:
##########
@@ -221,5 +224,5 @@ match the "ib" SSTable version
 
 [source,bash]
 ----
-include:example$find_sstables.sh[]
+include::../../examples/BASH/find_sstables.sh[]

Review Comment:
   ```suggestion
   include::example$BASH/find_sstables.sh[]
   ```



##########
doc/modules/cassandra/pages/operating/auditlogging.adoc:
##########
@@ -123,16 +136,25 @@ audit_logging_options:
 
 === enabled
 
-Audit logging is enabled by setting the `enabled` option to `true` in
-the `audit_logging_options` setting. 
+Control wether audit logging is enabled or disabled (default).

Review Comment:
   ```suggestion
   Control whether audit logging is enabled or disabled (default).
   ```



##########
doc/modules/cassandra/pages/operating/auditlogging.adoc:
##########
@@ -88,10 +88,23 @@ Common audit log entry types are one of the following:
 | ERROR | REQUEST_FAILURE
 |===
 
+== Availability and durability
+
+NOTE: Unlike data, audit log entries are not replicated
+
+For a given query, the corresponding audit entry is only stored on the coordinator node.
+For example, an ``INSERT`` in a keyspace with replication factor of 3 will produce an audit entry on one node, the coordinator who handled the request, and not on the two other nodes.
+For this reason, and depending on compliance requirements you must meet, you have to make sure that audig logs are stored on a non-ephemeral storage.

Review Comment:
   ```suggestion
   For this reason, and depending on compliance requirements you must meet, make sure that audit logs are stored on a non-ephemeral storage.
   ```



##########
doc/modules/cassandra/pages/architecture/snitch.adoc:
##########
@@ -2,17 +2,17 @@
 
 In cassandra, the snitch has two functions:
 
-* it teaches Cassandra enough about your network topology to route
+* It teaches Cassandra enough about your network topology to route

Review Comment:
   Generally, with a colon in the previous sentence, you use lowercase to start the bullet item.



##########
doc/modules/cassandra/pages/architecture/storage_engine.adoc:
##########
@@ -3,17 +3,17 @@
 [[commit-log]]
 == CommitLog
 
-Commitlogs are an append only log of all mutations local to a Cassandra
+Commitlogs are an append-only log of all mutations local to a Cassandra
 node. Any data written to Cassandra will first be written to a commit
 log before being written to a memtable. This provides durability in the
 case of unexpected shutdown. On startup, any mutations in the commit log
 will be applied to memtables.
 
-All mutations write optimized by storing in commitlog segments, reducing
-the number of seeks needed to write to disk. Commitlog Segments are
-limited by the `commitlog_segment_size` option, once the size is
+All mutations are write-optimized by storing in commitlog segments, reducing
+the number of seeks needed to write to disk. Commitlog segments are
+limited by the `commitlog_segment_size` option. Once the size is
 reached, a new commitlog segment is created. Commitlog segments can be
-archived, deleted, or recycled once all its data has been flushed to
+archived, deleted, or recycled once all their data has been flushed to

Review Comment:
   ```suggestion
   archived, deleted, or recycled once all the data has been flushed to
   ```



##########
doc/modules/cassandra/pages/operating/auditlogging.adoc:
##########
@@ -186,52 +210,101 @@ excluded_categories: DDL, DML, QUERY, PREPARE
 Users to audit log are set with the `included_users` and `excluded_users` options. 
 The `included_users` option specifies a comma-separated list of users to include explicitly.
 The `excluded_users` option specifies a comma-separated list of users to exclude explicitly.
-By default all users are included, and no users are excluded. 
+By default, all users are included, and no users are excluded.
 
 [source, yaml]
 ----
 included_users: 
 excluded_users: john, mary
 ----
 
+[[roll_cycle]]
 === roll_cycle
 
 The ``roll_cycle`` defines the frequency with which the audit log segments are rolled.
-Supported values are ``HOURLY`` (default), ``MINUTELY``, and ``DAILY``.
+Supported values are:
+
+- ``MINUTELY``
+- ``FIVE_MINUTELY``
+- ``TEN_MINUTELY``
+- ``TWENTY_MINUTELY``
+- ``HALF_HOURLY``
+- ``HOURLY`` (default)
+- ``TWO_HOURLY``
+- ``FOUR_HOURLY``
+- ``SIX_HOURLY``
+- ``DAILY``
+
 For example: ``roll_cycle: DAILY``
 
+WARNING: Read the following paragraph when changing ``roll_cycle`` on a production node.
+
+With the `BinLogger` implementation, any attempt to modify the roll cycle on a node where audit logging was previously enabled will fail silentely due to https://github.com/OpenHFT/Chronicle-Queue[Chronicle Queue] roll cycle inference mechanism (even if you delete the ``metadata.cq4t`` file).
+
+Here is an example of such an override visible in Cassandra logs:
+----
+INFO  [main] <DATE TIME> BinLog.java:420 - Attempting to configure bin log: Path: /path/to/audit Roll cycle: TWO_HOURLY [...]
+WARN  [main] <DATE TIME> SingleChronicleQueueBuilder.java:477 - Overriding roll cycle from TWO_HOURLY to FIVE_MINUTE
+----
+
+In order to change ``roll_cycle`` on a node, you have to:
+
+1. Stop Cassandra
+2. Move or offload all audit logs somewhere else (in a safe and durable location)
+3. Restart Cassandra.
+4. Check Cassandra logs
+5. Make sure that audit log filenames under ``audit_logs_dir`` correspond to the new roll cycle.
+
 === block
 
 The ``block`` option specifies whether audit logging should block writing or drop log records if the audit logging falls behind. Supported boolean values are ``true`` (default) or ``false``.
-For example: ``block: false`` to drop records
+
+For example: ``block: false`` to drop records (e.g. if audit is used for troobleshooting)
+
+For regulatory compliance purpose, it's a good practice to explicitely set ``block: true`` to prevent any regression in case of future default value change.

Review Comment:
   ```suggestion
   For regulatory compliance purposes, it's a good practice to explicitly set ``block: true`` to prevent any regression in case of future default value change.
   ```



##########
doc/modules/cassandra/pages/operating/auditlogging.adoc:
##########
@@ -88,10 +88,23 @@ Common audit log entry types are one of the following:
 | ERROR | REQUEST_FAILURE
 |===
 
+== Availability and durability
+
+NOTE: Unlike data, audit log entries are not replicated
+
+For a given query, the corresponding audit entry is only stored on the coordinator node.
+For example, an ``INSERT`` in a keyspace with replication factor of 3 will produce an audit entry on one node, the coordinator who handled the request, and not on the two other nodes.
+For this reason, and depending on compliance requirements you must meet, you have to make sure that audig logs are stored on a non-ephemeral storage.
+
+You can achieve custom needs with <<archive_command>> option.

Review Comment:
   ```suggestion
   You can achieve custom needs with the <<archive_command>> option.
   ```



##########
doc/modules/cassandra/pages/architecture/storage_engine.adoc:
##########
@@ -3,17 +3,17 @@
 [[commit-log]]
 == CommitLog
 
-Commitlogs are an append only log of all mutations local to a Cassandra
+Commitlogs are an append-only log of all mutations local to a Cassandra
 node. Any data written to Cassandra will first be written to a commit
 log before being written to a memtable. This provides durability in the
 case of unexpected shutdown. On startup, any mutations in the commit log
 will be applied to memtables.
 
-All mutations write optimized by storing in commitlog segments, reducing
-the number of seeks needed to write to disk. Commitlog Segments are
-limited by the `commitlog_segment_size` option, once the size is
+All mutations are write-optimized by storing in commitlog segments, reducing
+the number of seeks needed to write to disk. Commitlog segments are
+limited by the `commitlog_segment_size` option. Once the size is
 reached, a new commitlog segment is created. Commitlog segments can be
-archived, deleted, or recycled once all its data has been flushed to
+archived, deleted, or recycled once all their data has been flushed to

Review Comment:
   Don't anthropomorphize. a commit log is not a person. :-)



-- 
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: pr-unsubscribe@cassandra.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org