[
https://issues.apache.org/jira/browse/FLINK-30259?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ran Tao updated FLINK-30259:
----------------------------
Description:
The code of some modules of the current Flink project uses the 'assert' keyword
of java to do checking, which actually depends on the enablement of the
-enableassertions (-ea) option (default is false, which means some assert code
can not work), otherwise it may lead to unexpected behavior. In fact, flink
already has a mature Preconditions tool, we can use it to replace 'assert'
keyword. it is more clean and consistent with flink.
The following is an example of some snippets.
RowDataPrintFunction
{code:java}
@Override
public void invoke(RowData value, Context context) {
Object data = converter.toExternal(value);
assert data != null;
writer.write(data.toString());
}
{code}
e.g. if assert not enable,data.toString() will cause NPE.
KubernetesUtils
{code:java}
public static KubernetesConfigMap checkConfigMaps(
List<KubernetesConfigMap> configMaps, String expectedConfigMapName)
{
assert (configMaps.size() == 1);
assert (configMaps.get(0).getName().equals(expectedConfigMapName));
return configMaps.get(0);
}
{code}
e.g. if assert not enable,configMaps.get(0)will cause NPE.
RocksDBOperationUtils
{code:java}
if (memoryConfig.isUsingFixedMemoryPerSlot()) {
assert memoryConfig.getFixedMemoryPerSlot() != null;
logger.info("Getting fixed-size shared cache for RocksDB.");
return memoryManager.getExternalSharedMemoryResource(
FIXED_SLOT_MEMORY_RESOURCE_ID,
allocator,
// if assert not enable, here will cause NPE.
memoryConfig.getFixedMemoryPerSlot().getBytes());
} else {
logger.info("Getting managed memory shared cache for RocksDB.");
return memoryManager.getSharedMemoryResourceForManagedMemory(
MANAGED_MEMORY_RESOURCE_ID, allocator, memoryFraction);
}
{code}
e.g. if assert not enable,
RocksDBOperationUtils#memoryConfig.getFixedMemoryPerSlot().getBytes()) will
cause NPE.
*Note:* many calcite-releated classes such as flink hive parser(Most are
located in flink-connector-hive) use assert many places. we will not fix these
classes.
we need to keep these classes consistent with the assert used in calcite. We
just fix flink classes.
was:
The code of some modules of the current Flink project uses the 'assert' keyword
of java to do checking, which actually depends on the enablement of the
-enableassertions (-ea) option (default is false, which means some assert code
can not work), otherwise it may lead to unexpected behavior. In fact, flink
already has a mature Preconditions tool, we can use it to replace 'assert'
keyword. it is more clean and consistent with flink.
The following is an example of some snippets.
RowDataPrintFunction
{code:java}
@Override
public void invoke(RowData value, Context context) {
Object data = converter.toExternal(value);
assert data != null;
writer.write(data.toString());
}
{code}
e.g. if assert not enable,data.toString() will cause NPE.
KubernetesUtils
{code:java}
public static KubernetesConfigMap checkConfigMaps(
List<KubernetesConfigMap> configMaps, String expectedConfigMapName)
{
assert (configMaps.size() == 1);
assert (configMaps.get(0).getName().equals(expectedConfigMapName));
return configMaps.get(0);
}
{code}
e.g. if assert not enable,configMaps.get(0)will cause NPE.
RocksDBOperationUtils
{code:java}
if (memoryConfig.isUsingFixedMemoryPerSlot()) {
assert memoryConfig.getFixedMemoryPerSlot() != null;
logger.info("Getting fixed-size shared cache for RocksDB.");
return memoryManager.getExternalSharedMemoryResource(
FIXED_SLOT_MEMORY_RESOURCE_ID,
allocator,
// if assert not enable, here will cause NPE.
memoryConfig.getFixedMemoryPerSlot().getBytes());
} else {
logger.info("Getting managed memory shared cache for RocksDB.");
return memoryManager.getSharedMemoryResourceForManagedMemory(
MANAGED_MEMORY_RESOURCE_ID, allocator, memoryFraction);
}
{code}
e.g. if assert not enable,
RocksDBOperationUtils#memoryConfig.getFixedMemoryPerSlot().getBytes()) will
cause NPE.
Note: many calcite classes such as flink hive parser use assert many places. we
will not fix these classes. we just fix flink classes.
> Use flink Preconditions Util instead of uncertain Assert keyword to do
> checking
> -------------------------------------------------------------------------------
>
> Key: FLINK-30259
> URL: https://issues.apache.org/jira/browse/FLINK-30259
> Project: Flink
> Issue Type: Improvement
> Components: Deployment / Kubernetes, Formats (JSON, Avro, Parquet,
> ORC, SequenceFile), Table SQL / Planner
> Affects Versions: 1.16.0
> Reporter: Ran Tao
> Priority: Major
>
> The code of some modules of the current Flink project uses the 'assert'
> keyword of java to do checking, which actually depends on the enablement of
> the -enableassertions (-ea) option (default is false, which means some assert
> code can not work), otherwise it may lead to unexpected behavior. In fact,
> flink already has a mature Preconditions tool, we can use it to replace
> 'assert' keyword. it is more clean and consistent with flink.
> The following is an example of some snippets.
> RowDataPrintFunction
> {code:java}
> @Override
> public void invoke(RowData value, Context context) {
> Object data = converter.toExternal(value);
> assert data != null;
> writer.write(data.toString());
> }
> {code}
> e.g. if assert not enable,data.toString() will cause NPE.
> KubernetesUtils
> {code:java}
> public static KubernetesConfigMap checkConfigMaps(
> List<KubernetesConfigMap> configMaps, String
> expectedConfigMapName) {
> assert (configMaps.size() == 1);
> assert (configMaps.get(0).getName().equals(expectedConfigMapName));
> return configMaps.get(0);
> }
> {code}
> e.g. if assert not enable,configMaps.get(0)will cause NPE.
> RocksDBOperationUtils
> {code:java}
> if (memoryConfig.isUsingFixedMemoryPerSlot()) {
> assert memoryConfig.getFixedMemoryPerSlot() != null;
> logger.info("Getting fixed-size shared cache for RocksDB.");
> return memoryManager.getExternalSharedMemoryResource(
> FIXED_SLOT_MEMORY_RESOURCE_ID,
> allocator,
> // if assert not enable, here will cause NPE.
> memoryConfig.getFixedMemoryPerSlot().getBytes());
> } else {
> logger.info("Getting managed memory shared cache for
> RocksDB.");
> return memoryManager.getSharedMemoryResourceForManagedMemory(
> MANAGED_MEMORY_RESOURCE_ID, allocator,
> memoryFraction);
> }
> {code}
> e.g. if assert not enable,
> RocksDBOperationUtils#memoryConfig.getFixedMemoryPerSlot().getBytes()) will
> cause NPE.
> *Note:* many calcite-releated classes such as flink hive parser(Most are
> located in flink-connector-hive) use assert many places. we will not fix
> these classes.
> we need to keep these classes consistent with the assert used in calcite. We
> just fix flink classes.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
